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

Load+save things other than filepaths, and specifically support netCDF4.Dataset-s #5024

Closed

Conversation

pp-mo
Copy link
Member

@pp-mo pp-mo commented Oct 12, 2022

Replaces the iris-load changes part of #4835,
So, re-presented as a separate issue which should be much simpler.

TBD: reform + add the other parts of that logic, in subsequent PRs.

NOTE:
I think that, whilst we are targetting a feature-branch, it isn't generally appropriate to add whatsnew-s.
So, we can sort out how to announce+explain it all when we merge back the FB.
Deferring the changes also probably avoids a whole lot of trivial conflicts whenever we do merge main-to-FB or vice versa.

@pp-mo
Copy link
Member Author

pp-mo commented Oct 12, 2022

NOTE: some of this will need re-visiting,
since @TomekTrzeciak has pointed out that it will be better to 'hide' our ugly netCDF.Dataset adapter class for internal usage only.
In which case, we will be passing a cleaner 'abstract netcdf data' object in place of an object mimicking a Dataset, and the loader will have to recognise those things as well.
But I still think that loading from an actual open Dataset makes sense -- you can even do useful things with that.
So IMHO this will do for a start.

@@ -587,7 +587,7 @@ def load_cubes(file_sources, callback=None, constraints=None):
warnings.warn("{}".format(e))

# Perform any user registered callback function.
cube = run_callback(callback, cube, cf_var, cf.filename)
cube = run_callback(callback, cube, cf_var, file_source)
Copy link
Member Author

@pp-mo pp-mo Oct 12, 2022

Choose a reason for hiding this comment

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

I think this makes more sense.
I believe it is consistent with what we do for UM "structured loading"
see "Use of load callbacks" in structured_um_loading api docs
-- though in this case it is the filename arg that is different to usual, rather than the field one.

@pp-mo pp-mo mentioned this pull request Oct 21, 2022
@pp-mo
Copy link
Member Author

pp-mo commented Mar 14, 2023

Just re-raising this, and put into Iris 3.5 project, since some such feature will be important to the proposed solutions for #4994
But we should remain open to changing how this is implemented.

@pp-mo
Copy link
Member Author

pp-mo commented Mar 27, 2023

Closing in favour of #5214

Which now targets main, and required a lot of rebasing !
The F-B rather hid the amount of work that was required to integrate these changes, but I don't see the point in updating the F-B since plans have changed : we now intend to host the "bridge" code outside the Iris repo.
I will also remove the F-B branch.

@pp-mo pp-mo closed this Mar 27, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: 🏁 Done - v3.6.0
Development

Successfully merging this pull request may close these issues.

None yet

1 participant