-
Notifications
You must be signed in to change notification settings - Fork 16
Implement implicit conversion, lazy loading and some performance improvements for netCDF backend #17
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
Implement implicit conversion, lazy loading and some performance improvements for netCDF backend #17
Conversation
…y instead of a python scalar (`int` or `float`)
DaanVanVugt
left a comment
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.
very nice, this will be useful
olivhoenen
left a comment
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.
Thanks @maarten-ic , both the lazy loading and conversion (test on _tor-->_phi) are working like a charm!
Could you maybe mention these briefly in the example for netcdf in the doc?
imas/test/test_nc_validation.py
Outdated
| # This one is valid: | ||
| NC2IDS(memfile, factory.core_profiles()).run() | ||
| ids = factory.core_profiles() | ||
| NC2IDS(memfile, ids, ids.metadata, None).run() |
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.
Does the run method need a Boolean (lazy) parameter?
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.
Good point, fixed that now. Also created #18, could you have a look at that?
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.
Yes, I will update the GitHub actions : #18
…thon into feature/lazy-load-from-netcdf
Good suggestion! I've added a table with supported (and unsupported) features with links to the sections with further details. Please check if this works for you :) |
olivhoenen
left a comment
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.
LGTM
prasad-sawantdesai
left a comment
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.
LGTM
This Pull Request implements the following improvements for the netCDF backend:
get(). This has the same limitations as the automatic conversion usingimas_core, as outlined in the documentation.N.B. automatic conversion during
put()is still not supported, since this is rarely needed and easily worked around.IDSPrimitive.value.setterlogic when loading data from a netCDF file (the same optimization was already in place for reading from animas_corebackend).