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.

Sign up for GitHub

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

Jump to bottom

Fix unit power regex for parentheses handling #2380

Merged
merged 1 commit into from
Mar 8, 2022
Merged

Fix unit power regex for parentheses handling #2380

merged 1 commit into from
Mar 8, 2022

Conversation

jthielen
Copy link
Collaborator

@jthielen jthielen commented Mar 6, 2022 •
edited
Loading

Description Of Changes

MetPy's CF/UDUNITS-style power handling regex failed on parentheses (as noted in #1362 (comment) and #1362 (comment) ). This fixes it and adds tests against those previously failing examples!

Checklist

  • Closes
  • Tests added
  • Fully documented

@jthielen jthielen added Type: Bug Something is not working like it should Area: Units Pertains to unit information labels Mar 6, 2022
@jthielen jthielen added this to the 1.3.0 milestone Mar 6, 2022
@jthielen jthielen requested a review from a team as a code owner March 6, 2022 23:41
@jthielen jthielen requested review from dopplershift and removed request for a team March 6, 2022 23:41
@jthielen jthielen mentioned this pull request Mar 6, 2022
Merged
3 tasks
@jthielen jthielen force-pushed the fix-udunits-parens branch from 63ae945 to 233d7ca Compare March 7, 2022 20:30
@jthielen
Copy link
Collaborator Author

jthielen commented Mar 7, 2022 •
edited
Loading

After closer inspection with cf_units, my comment in #1362 (comment) was actually wrong! We were actually right previously, so this PR updates the list of unit strings accordingly. All the more motivation to actually test against UDUNITS properly!

Also, some strange Pint behavior was uncovered, so I raised hgrecco/pint#1485 upstream.

@jthielen jthielen force-pushed the fix-udunits-parens branch from 233d7ca to 5640888 Compare March 7, 2022 20:33
dopplershift
dopplershift approved these changes Mar 7, 2022
View reviewed changes
Copy link
Member

@dopplershift dopplershift 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 doing that. This should make it easier to update when we find future things not working right.

@dopplershift dopplershift merged commit 4d5c627 into Unidata:main Mar 8, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dopplershift dopplershift dopplershift approved these changes

Assignees
No one assigned
Labels
Area: Units Pertains to unit information Type: Bug Something is not working like it should
Projects
None yet
Milestone
1.3.0
Development

Successfully merging this pull request may close these issues.
2 participants
@jthielen @dopplershift