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 unit converter problem with units = 1 #460

Merged
merged 1 commit into from
Jan 24, 2023

Conversation

gold2718
Copy link
Collaborator

Fix unit converter problem with units = 1

In order for the unit converter to be able to attempt a unit conversion where one variable has units = 1, a special case has been added where the digit, 1, is converted to the string one (a valid Python identifier).

User interface changes?: No

Fixes: #458, Unit converter does not handle a variable with units = 1

Testing:
unit tests: run with a new test added to test this functionality.
system tests: ./test/run_tests.sh (is that unit, system, or both?)
manual testing: NA
Note that var_action_test is a known fail for this PR.

@gold2718 gold2718 added capgen bugs, requests, etc. that involve ccpp_capgen bugfix Fix for issue with 'bug' label. labels Dec 23, 2022
@gold2718 gold2718 requested a review from mkavulich December 23, 2022 15:35
@gold2718 gold2718 requested a review from climbfuji as a code owner December 23, 2022 15:35
@gold2718 gold2718 self-assigned this Dec 23, 2022
@gold2718
Copy link
Collaborator Author

Replaces #459

@gold2718
Copy link
Collaborator Author

Note that I did no add any conversion between 1 and any other unit (including None) but at least now you get the intended error from the unit converter.

Copy link
Collaborator

@climbfuji climbfuji 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 fixing the unit conversion problem, and for adding all these new tests. Wondering if we should have a corresponding fix in prebuild for the time being.

@ligiabernardet
Copy link
Collaborator

Yes. Perhaps @mkavulich can implement it once re returns from leave.

@mkavulich mkavulich merged commit dc6458e into NCAR:feature/capgen Jan 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bugfix Fix for issue with 'bug' label. capgen bugs, requests, etc. that involve ccpp_capgen
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants