Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix test suite to account for beta numpy versions #70

Merged
merged 1 commit into from
Aug 29, 2023

Conversation

mgorny
Copy link
Contributor

@mgorny mgorny commented Aug 29, 2023

  • I have added the tests to cover my changes.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.

Summary

Fix the numpy version check in the test suite to account for the possibility that numpy.__version__ may contain letters.

Details and comments

Prior to this change, running tests on numpy 1.26.0b1 would result in the following error:

test/python/algorithms/optimizers/test_optimizers_scikitquant.py:72: in TestOptimizers
    tuple(map(int, numpy.__version__.split("."))) >= (1, 24, 0),
E   ValueError: invalid literal for int() with base 10: '0b1'

Since the third component is matched against zero, the simplest solution is to check only the first two components, since we can expect that the first two split elements will be integers.

This is synced from the equivalent qiskit change:
Qiskit/qiskit#10723

Fix the numpy version check in the test suite to account
for the possibility that `numpy.__version__` may contain letters.
Prior to this change, running tests on numpy 1.26.0b1 would result
in the following error:

```
test/python/algorithms/optimizers/test_optimizers_scikitquant.py:72: in TestOptimizers
    tuple(map(int, numpy.__version__.split("."))) >= (1, 24, 0),
E   ValueError: invalid literal for int() with base 10: '0b1'
```

Since the third component is matched against zero, the simplest solution
is to check only the first two components, since we can expect that
the first two split elements will be integers.

This is synced from the equivalent qiskit change:
Qiskit/qiskit#10723
Copy link
Member

@woodsp-ibm woodsp-ibm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for replicating the fix here too.

@CLAassistant
Copy link

CLAassistant commented Aug 29, 2023

CLA assistant check
All committers have signed the CLA.

@coveralls
Copy link

Pull Request Test Coverage Report for Build 6014844995

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 90.038%

Totals Coverage Status
Change from base Build 5964934865: 0.0%
Covered Lines: 6444
Relevant Lines: 7157

💛 - Coveralls

@woodsp-ibm
Copy link
Member

@mgorny You will need to agree to the CLA here in qiskit-community organization too

@mgorny
Copy link
Contributor Author

mgorny commented Aug 29, 2023

Done.

@mergify mergify bot merged commit 4464447 into qiskit-community:main Aug 29, 2023
15 checks passed
@mgorny mgorny deleted the numpy-beta branch August 29, 2023 18:37
@mgorny
Copy link
Contributor Author

mgorny commented Aug 29, 2023

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants