-
Notifications
You must be signed in to change notification settings - Fork 283
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
Mesh nonexperimental #6061
Mesh nonexperimental #6061
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #6061 +/- ##
==========================================
- Coverage 89.75% 89.69% -0.07%
==========================================
Files 90 91 +1
Lines 22981 22997 +16
Branches 5026 5026
==========================================
Hits 20627 20627
- Misses 1624 1640 +16
Partials 730 730 ☔ View full report in Codecov by Sentry. |
4707612
to
c545b62
Compare
Status update 2024-07-19 ~15:30lots of conflicts with main now. |
c545b62
to
5b4e1f3
Compare
Status update 2024-07-19 ~1900rebased + resolved conflicts Status 2024-07-22now up-to-date with latest main, and passing. |
99308cd
to
15d4f84
Compare
Status 2024-07-22 ~17:30Found that benchmark code also needs updating to new module path AND removal of PARSE_UGRID_ON_LOAD What's left to do
|
?ready for review?This roughly hangs together + worth reviewing now. Though notably, the benchmarks need updating, and I have plans to refactor + (further) rename the API. |
Hmm benchmarks show a worrying thing.. It's not a fluke, I only did this because of #6075 |
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.
@pp-mo Awesome effort 👍
Just a few TODO
comments as reminders to address afterwards 👍
As an aside, I noticed that we're pulling the use of the logger
across from iris.experimental.ugrid
into the iris
core. Not sure about our current appetite on the use of logging ATM, but seems like tech debt to me, but that's an out of scope discussion.
Also, with regards to API, rebranding iris.ugrid
as iris.mesh
is fine from my perspective to be more future UGRID agnostic. However, that means we'd have iris.mesh.mesh
which is less than ideal.
In this case, I'd recommend incorporating the iris.ugrid.mesh
into __init__.py
of iris.ugrid
and then rebrand 👍
Otherwise, all good, but there is an iris.tests.stock.__init__
comment to check and service 👍
OK finally I pushed, think I have covered those changes. @bjlittle please re-review ! |
Replace experiment.ugrid, including docstrings and imports. Fix test_ParseUgridOnLoad Fix ugrid.load. Remove PARSE_UGRID from t/i/ugrid/test_ugrid_save Remove PARSE_UGRID from t/u/ff/nc/saver/test_save Remove PARSE_UGRID from t/i/exp/geovista/(both) Remove PARSE_UGRID from t/u/tests/stock/test_netcdf
Include alternative index workaround.
f8b7644
to
2d614b8
Compare
Thanks so much @bjlittle And the good news ... the next one might be worse ! |
@pp-mo Bring it! You da man 😄 |
* Move all iris.experimental.ugrid to iris.ugrid. Replace experiment.ugrid, including docstrings and imports. Fix test_ParseUgridOnLoad Fix ugrid.load. Remove PARSE_UGRID from t/i/ugrid/test_ugrid_save Remove PARSE_UGRID from t/u/ff/nc/saver/test_save Remove PARSE_UGRID from t/i/exp/geovista/(both) Remove PARSE_UGRID from t/u/tests/stock/test_netcdf * Fix type of mesh in iris.tests.stock * Fix Mesh -> MeshXY in experimental.ugrid * Remove PARSE_UGRID_ON_LOAD and ParseUGridOnLoad, leave in iris.experimental.ugrid only. missed * Replace old-style mesh api in benchmarks. * Move misplaced test + fix obsolete PARSE_UGRID usage. * Minimal doctest fixes * Remove obsolete PARSE_UGRID in iris.ugrid * Move all ugrid.metadata into common.metadata. Removed obsolete tests.unit.metadata * Rename iris.ugrid to iris.mesh fix Fix iris.mesh import in tests/stock/__init__ * Move all iris.mesh.save into iris.fileformats.netcdf.save Fix experimental.ugrid import of save_mesh * Fix docs for iris.ugrid -> iris.mesh * Rename iris.mesh.mesh as iris.mesh.components * Tidy imports in experimental.ugrid * Rebrand so mesh.MeshXY is presented as experimental.ugrid.Mesh. * Move ugrid loading support code from iris.mesh to iris.netcdf, and its tests likewise * Fix circular import. * Move mesh.cf code into fileformats.cf, and tests likewise Fix imports in cf ugrid tests. * Reinclude mesh-load functions in iris.mesh; break circular imports * Small fixes to mesh documentation * Added whatsnew * Complete+remove remaining 'TODO's from #6061 * Odd docstring and comment corrections.
* main: Mesh nonexperimental extra (SciTools#6077) Mesh nonexperimental (SciTools#6061) # Conflicts: # lib/iris/experimental/ugrid/cf.py
* upstream/main: Load performance improvement (ignoring UGRID) (SciTools#6088) Adding monthly and yearly arguments to `guess_bounds` (SciTools#6090) Mesh nonexperimental extra (SciTools#6077) Mesh nonexperimental (SciTools#6061) Bump scitools/workflows from 2024.07.4 to 2024.07.5 (SciTools#6076) Updated environment lockfiles (SciTools#6068) Enable UGRID loading always; deprecate PARSE_UGRID_ON_LOAD. (SciTools#6054) Bump scitools/workflows from 2024.07.3 to 2024.07.4 (SciTools#6071)
Addresses #6057
WIP will need some rebase when outstanding changes are merged :
Mesh
toMeshXY
#6052Also remaining
tidying to-do :
iris.mesh
andiris.mesh.utils
For simplicity, I'm going to defer that to another PR : see #6073