Skip to content

Update for latest version of gimli.units #169

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

Merged
merged 10 commits into from
Oct 10, 2024
Merged

Conversation

mdpiper
Copy link
Member

@mdpiper mdpiper commented Oct 2, 2024

The latest version of gimli.units (v0.3.2) includes some significant changes that are breaking pymt. This PR is an attempt to update pymt to work with gimli.units. I'm trying to make as few changes as possible to 1) get the tests passing again and 2) ensure that gimli.units is being used correctly in pymt.

This fixes #168.

@mdpiper
Copy link
Member Author

mdpiper commented Oct 2, 2024

A lot of files were updated, but most of the changes were updates from black and flake8 to get get the tests passing.

I also modified the CI workflows to get them to the point of running the unit tests.

@mdpiper
Copy link
Member Author

mdpiper commented Oct 2, 2024

@mcflugen Would you mind taking a look at this, please? The last few tests that are failing are from changes in gimli.units, and I'm not sure how best to proceed.

=========================== short test summary info ============================
FAILED tests/framework/test_bmi_var_units.py::test_incompatible_units - gimli._udunits2.UdunitsError: udunits error (6)
FAILED tests/framework/test_bmi_var_units.py::test_bad_units - gimli._udunits2.UdunitsError: udunits error (11)
FAILED tests/test_units.py::test_get_xml - AttributeError: type object 'UnitSystem' has no attribute 'get_xml_path'
FAILED tests/test_units.py::test_default_system - AttributeError: 'str' object has no attribute 'is_file'
FAILED tests/test_units.py::test_user_system - AttributeError: 'str' object has no attribute 'is_file'
FAILED tests/test_units.py::test_env_system - AttributeError: 'str' object has no attribute 'is_file'
FAILED tests/test_units.py::test_unit_converter_bad_from_units[not_a_unit-m] - gimli._udunits2.UdunitsError: udunits error (11)
FAILED tests/test_units.py::test_unit_converter_bad_from_units[m-not_a_unit] - gimli._udunits2.UdunitsError: udunits error (11)
FAILED tests/test_units.py::test_unit_converter_bad_from_units[not_a_unit-not_a_unit] - gimli._udunits2.UdunitsError: udunits error (11)
FAILED tests/test_units.py::test_unit_converter_incompatible_units - gimli._udunits2.UdunitsError: udunits error (6)
=========== 10 failed, 352 passed, 29 skipped, 160 warnings in 5.67s ===========

If you could fix these, or let me know how to fix them, I'd appreciate it.

This was referenced Oct 2, 2024
@mcflugen mcflugen force-pushed the mdpiper/update-for-gimli branch from 806ea54 to 556183d Compare October 3, 2024 05:52
@mcflugen
Copy link
Member

mcflugen commented Oct 3, 2024

@mdpiper I've fixed the failing gimli.units tests. Most of those tests were simply testing gimli, not pymt, and are, in fact, repeated almost verbatim in the unit tests of the gimli package. As such, I just removed them as they are redundant and don't belong in pymt.

NOTE: I've rebased this branch with master so you'll have to update your local branch before making any additional changes.

@mdpiper
Copy link
Member Author

mdpiper commented Oct 4, 2024

The notebook tests are failing because pymt_gipl and pymt_ecsimplesnow aren't built on osx_arm64 (#172). I've updated the ecsimplesnow feedstock and I'm waiting on pymt_ecsimplesnow. gipl isn't compiling, but I think I can fix it. I'll then go through the process of updating its feedstocks.

@mdpiper mdpiper marked this pull request as ready for review October 8, 2024 01:19
@mdpiper mdpiper requested a review from mcflugen October 8, 2024 01:54
Copy link
Member

@mcflugen mcflugen left a comment

Choose a reason for hiding this comment

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

👍

@mdpiper mdpiper merged commit 78cfd16 into master Oct 10, 2024
11 checks passed
@mdpiper mdpiper deleted the mdpiper/update-for-gimli branch October 10, 2024 15:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Error from gimli when importing pymt
2 participants