Fix asv problems #2065
Merged
Fix asv problems #2065
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
G-D-Petrov
requested review from
alexowens90,
willdealtry and
poodlewars
as code owners
#### Reference Issues/PRs <!--Example: Fixes #1234. See also #3456.--> #### What does this implement or fix? #### Any other comments? #### Checklist <details> <summary> Checklist for code changes... </summary> - [ ] Have you updated the relevant docstrings, documentation and copyright notice? - [ ] Is this contribution tested against [all ArcticDB's features](../docs/mkdocs/docs/technical/contributing.md)? - [ ] Do all exceptions introduced raise appropriate [error messages](https://docs.arcticdb.io/error_messages/)? - [ ] Are API changes highlighted in the PR description? - [ ] Is the PR labelled as enhancement or bug so it appears in autogenerated release notes? </details> <!-- Thanks for contributing a Pull Request to ArcticDB! Please ensure you have taken a look at: - ArcticDB's Code of Conduct: https://github.com/man-group/ArcticDB/blob/master/CODE_OF_CONDUCT.md - ArcticDB's Contribution Licensing: https://github.com/man-group/ArcticDB/blob/master/docs/mkdocs/docs/technical/contributing.md#contribution-licensing --> --------- Co-authored-by: Georgi Rusev <Georgi Rusev>
#### Reference Issues/PRs <!--Example: Fixes #1234. See also #3456.--> #### What does this implement or fix? #### Any other comments? #### Checklist <details> <summary> Checklist for code changes... </summary> - [ ] Have you updated the relevant docstrings, documentation and copyright notice? - [ ] Is this contribution tested against [all ArcticDB's features](../docs/mkdocs/docs/technical/contributing.md)? - [ ] Do all exceptions introduced raise appropriate [error messages](https://docs.arcticdb.io/error_messages/)? - [ ] Are API changes highlighted in the PR description? - [ ] Is the PR labelled as enhancement or bug so it appears in autogenerated release notes? </details> <!-- Thanks for contributing a Pull Request to ArcticDB! Please ensure you have taken a look at: - ArcticDB's Code of Conduct: https://github.com/man-group/ArcticDB/blob/master/CODE_OF_CONDUCT.md - ArcticDB's Contribution Licensing: https://github.com/man-group/ArcticDB/blob/master/docs/mkdocs/docs/technical/contributing.md#contribution-licensing --> --------- Co-authored-by: Georgi Rusev <Georgi Rusev>
IvoDD
approved these changes
Dec 16, 2024
| ''' | ||
| The test is meant to be executed with 3 times wider dataframe than the base test | ||
| ''' | ||
| if sys.version_info >= (3, 8): |
There was a problem hiding this comment.
I know this is from #2060 but since we upgrade the benchmarks to 3.11 we can remove this check?
There was a problem hiding this comment.
It can be removed or it can stay. Technically this line is correct - it will prevent code from execution if attempted in future too. Situationally no it can be removed, and removing will erase all possibilities to be executed that code with previous version, which is ok situationally for us
poodlewars
reviewed
Dec 16, 2024
| if (SLOW_TESTS): | ||
|
|
||
| def setup(self, cache: CachedDFGenerator, param: int): | ||
| if not SLOW_TESTS: |
There was a problem hiding this comment.
Not your code but wouldn't it be cleaner to lift the SLOW_TESTS conditions up to control whether we define the class at all? Rather than having them on each method?
There was a problem hiding this comment.
I am keen on merging this as is because I want to rebase my numpy branch with these changes, so I've created a separate issue (#2070) to track this and fix it in the future.
poodlewars
approved these changes
Dec 16, 2024
G-D-Petrov
added a commit
that referenced
this pull request
Dec 16, 2024
Fixes problems with publishing of ASV benchmarks This PR implements the following: - Upgrade the Python analysis flow and C++ tests to Python 3.11 - this is needed because some of the benchmarks need at least 3.10 - Fixes the problems with publishing of the benchmarks - because they can't evaluate np.inf correctly - Fixes LFS for BI benchmarks - Incorporates changes from #2060 See successful run [here](https://github.com/man-group/ArcticDB/actions/runs/12317667688/job/34380720877) (had to be started manually due to the changes to the workflow files) <details> <summary> Checklist for code changes... </summary> - [ ] Have you updated the relevant docstrings, documentation and copyright notice? - [ ] Is this contribution tested against [all ArcticDB's features](../docs/mkdocs/docs/technical/contributing.md)? - [ ] Do all exceptions introduced raise appropriate [error messages](https://docs.arcticdb.io/error_messages/)? - [ ] Are API changes highlighted in the PR description? - [ ] Is the PR labelled as enhancement or bug so it appears in autogenerated release notes? </details> <!-- Thanks for contributing a Pull Request to ArcticDB! Please ensure you have taken a look at: - ArcticDB's Code of Conduct: https://github.com/man-group/ArcticDB/blob/master/CODE_OF_CONDUCT.md - ArcticDB's Contribution Licensing: https://github.com/man-group/ArcticDB/blob/master/docs/mkdocs/docs/technical/contributing.md#contribution-licensing --> Fix CI issues Try to override the file during benchmarks Comment-out for testing Flaky asv (#2066) <!--Example: Fixes #1234. See also #3456.--> <details> <summary> Checklist for code changes... </summary> - [ ] Have you updated the relevant docstrings, documentation and copyright notice? - [ ] Is this contribution tested against [all ArcticDB's features](../docs/mkdocs/docs/technical/contributing.md)? - [ ] Do all exceptions introduced raise appropriate [error messages](https://docs.arcticdb.io/error_messages/)? - [ ] Are API changes highlighted in the PR description? - [ ] Is the PR labelled as enhancement or bug so it appears in autogenerated release notes? </details> <!-- Thanks for contributing a Pull Request to ArcticDB! Please ensure you have taken a look at: - ArcticDB's Code of Conduct: https://github.com/man-group/ArcticDB/blob/master/CODE_OF_CONDUCT.md - ArcticDB's Contribution Licensing: https://github.com/man-group/ArcticDB/blob/master/docs/mkdocs/docs/technical/contributing.md#contribution-licensing --> --------- Co-authored-by: Georgi Rusev <Georgi Rusev> Test with LFS checkout Fix lfs install step Remove unnecessary check Fix code coverage flow try to use ec2 runner for code cov Test without the port Fix for 311
G-D-Petrov
added a commit
that referenced
this pull request
Dec 17, 2024
Fixes problems with publishing of ASV benchmarks This PR implements the following: - Upgrade the Python analysis flow and C++ tests to Python 3.11 - this is needed because some of the benchmarks need at least 3.10 - Fixes the problems with publishing of the benchmarks - because they can't evaluate np.inf correctly - Fixes LFS for BI benchmarks - Incorporates changes from #2060 See successful run [here](https://github.com/man-group/ArcticDB/actions/runs/12317667688/job/34380720877) (had to be started manually due to the changes to the workflow files) <details> <summary> Checklist for code changes... </summary> - [ ] Have you updated the relevant docstrings, documentation and copyright notice? - [ ] Is this contribution tested against [all ArcticDB's features](../docs/mkdocs/docs/technical/contributing.md)? - [ ] Do all exceptions introduced raise appropriate [error messages](https://docs.arcticdb.io/error_messages/)? - [ ] Are API changes highlighted in the PR description? - [ ] Is the PR labelled as enhancement or bug so it appears in autogenerated release notes? </details> <!-- Thanks for contributing a Pull Request to ArcticDB! Please ensure you have taken a look at: - ArcticDB's Code of Conduct: https://github.com/man-group/ArcticDB/blob/master/CODE_OF_CONDUCT.md - ArcticDB's Contribution Licensing: https://github.com/man-group/ArcticDB/blob/master/docs/mkdocs/docs/technical/contributing.md#contribution-licensing --> Fix CI issues Try to override the file during benchmarks Comment-out for testing Flaky asv (#2066) <!--Example: Fixes #1234. See also #3456.--> <details> <summary> Checklist for code changes... </summary> - [ ] Have you updated the relevant docstrings, documentation and copyright notice? - [ ] Is this contribution tested against [all ArcticDB's features](../docs/mkdocs/docs/technical/contributing.md)? - [ ] Do all exceptions introduced raise appropriate [error messages](https://docs.arcticdb.io/error_messages/)? - [ ] Are API changes highlighted in the PR description? - [ ] Is the PR labelled as enhancement or bug so it appears in autogenerated release notes? </details> <!-- Thanks for contributing a Pull Request to ArcticDB! Please ensure you have taken a look at: - ArcticDB's Code of Conduct: https://github.com/man-group/ArcticDB/blob/master/CODE_OF_CONDUCT.md - ArcticDB's Contribution Licensing: https://github.com/man-group/ArcticDB/blob/master/docs/mkdocs/docs/technical/contributing.md#contribution-licensing --> --------- Co-authored-by: Georgi Rusev <Georgi Rusev> Test with LFS checkout Fix lfs install step Remove unnecessary check Fix code coverage flow try to use ec2 runner for code cov Test without the port Fix for 311 test different path Test with fast tests only
alexowens90
pushed a commit
that referenced
this pull request
Dec 20, 2024
Fixes problems with publishing of ASV benchmarks This PR implements the following: - Upgrade the Python analysis flow and C++ tests to Python 3.11 - this is needed because some of the benchmarks need at least 3.10 - Fixes the problems with publishing of the benchmarks - because they can't evaluate np.inf correctly - Fixes LFS for BI benchmarks - Incorporates changes from #2060 See successful run [here](https://github.com/man-group/ArcticDB/actions/runs/12317667688/job/34380720877) (had to be started manually due to the changes to the workflow files) <details> <summary> Checklist for code changes... </summary> - [ ] Have you updated the relevant docstrings, documentation and copyright notice? - [ ] Is this contribution tested against [all ArcticDB's features](../docs/mkdocs/docs/technical/contributing.md)? - [ ] Do all exceptions introduced raise appropriate [error messages](https://docs.arcticdb.io/error_messages/)? - [ ] Are API changes highlighted in the PR description? - [ ] Is the PR labelled as enhancement or bug so it appears in autogenerated release notes? </details> <!-- Thanks for contributing a Pull Request to ArcticDB! Please ensure you have taken a look at: - ArcticDB's Code of Conduct: https://github.com/man-group/ArcticDB/blob/master/CODE_OF_CONDUCT.md - ArcticDB's Contribution Licensing: https://github.com/man-group/ArcticDB/blob/master/docs/mkdocs/docs/technical/contributing.md#contribution-licensing -->
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Reference Issues/PRs
Fixes problems with publishing of ASV benchmarks
What does this implement or fix?
This PR implements the following:
See successful run here (had to be started manually due to the changes to the workflow files)
Any other comments?
Checklist
Checklist for code changes...