-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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 version parsing issue in bloch.py. #12928
Conversation
One or more of the following people are relevant to this code:
|
# This version pattern is taken from the pypa packaging project: | ||
# https://github.com/pypa/packaging/blob/21.3/packaging/version.py#L223-L254 | ||
# which is dual licensed Apache 2.0 and BSD see the source for the original | ||
# authors and other details | ||
VERSION_PATTERN = ( | ||
"^" | ||
+ r""" | ||
v? | ||
(?: | ||
(?:(?P<epoch>[0-9]+)!)? # epoch | ||
(?P<release>[0-9]+(?:\.[0-9]+)*) # release segment | ||
(?P<pre> # pre-release | ||
[-_\.]? | ||
(?P<pre_l>(a|b|c|rc|alpha|beta|pre|preview)) | ||
[-_\.]? | ||
(?P<pre_n>[0-9]+)? | ||
)? | ||
(?P<post> # post release | ||
(?:-(?P<post_n1>[0-9]+)) | ||
| | ||
(?: | ||
[-_\.]? | ||
(?P<post_l>post|rev|r) | ||
[-_\.]? | ||
(?P<post_n2>[0-9]+)? | ||
) | ||
)? | ||
(?P<dev> # dev release | ||
[-_\.]? | ||
(?P<dev_l>dev) | ||
[-_\.]? | ||
(?P<dev_n>[0-9]+)? | ||
)? | ||
) | ||
(?:\+(?P<local>[a-z0-9]+(?:[-_\.][a-z0-9]+)*))? # local version | ||
""" | ||
+ "$" | ||
) | ||
VERSION_PATTERN_REGEX = re.compile(VERSION_PATTERN, re.VERBOSE | re.IGNORECASE) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we just import this from qpy it seems a bit weird to duplicate this a 4th time in the code: https://github.com/search?q=repo%3AQiskit%2Fqiskit%20VERSION_PATTERN&type=code
but at the end of the day I guess it doesn't matter too much.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Personally, I would rather just duplicate it unless we move it to a common location. I think importing from qpy
would be weirder in terms of module dependencies. It really shouldn't change anyway since it's not our code.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we should look at consolidating these down to one shared copy somewhere, but we can save that for a separate PR. I'm fine to add another copy of this here in the medium term to get this fix into 1.2.0.
Pull Request Test Coverage Report for Build 10306985942Details
💛 - Coveralls |
(cherry picked from commit 61adcf9)
(cherry picked from commit 61adcf9) Co-authored-by: Kevin Hartman <[email protected]>
Summary
Adds more robust version parsing in
bloch.py
, which we use to check the version ofmatplotlib
. The latest version ofmatplotlib
ismatplotlib 3.9.1.post1
and causes the old code to fail.Details and comments
This version regex stuff is taken from our
qpy
module.