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

new version of functioning xarray IO interactions #1060

Open
wants to merge 15 commits into
base: master
Choose a base branch
from

Conversation

Boorhin
Copy link
Contributor

@Boorhin Boorhin commented Jan 31, 2023

I have integrated all the changes and minor bug corrections I observed
I can (in my setup) read and write CF and UGRID compliants files
The only thing that I had to modify that may have an impact is a line in base_model.py that wants to import the file before you write it really.
It is meant as a source of inspiration more than an all rounder. It considerably accelerates my usage by reading directly zarr through xarray and writing trajectory files in xarray with daskarray buffers.

Copy link
Contributor Author

@Boorhin Boorhin left a comment

Choose a reason for hiding this comment

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

cleaned up

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ok that's a mistake I should push this file back?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

brought back in
@knutfrode
Copy link
Collaborator

Not sure why it cannot be installed, perhaps some cache from earlier. My own PRs yesterday passed through fine.
Perhaps you can make a new, clean PR, starting from latest on master?

Also it is not toally clear what this brings, can you make an example script for the gallery demonstrating usage?
And btw, it is possible to open Zarr datasets with Xarray and provide to reader_netCDF_CF_generic. So it should not be necessary with dedicated readers for Zarr specifically.

@Boorhin
Copy link
Contributor Author

Boorhin commented Jun 16, 2023

I removed the specific zarr reader as it was an old file.
I don't think merging into the netcdf reader is a good idea as it is not using xarray
The reader specific to xarray uses xarray methods that are not exactly the same as netcdf. You can open a netcdf with it if you want. There is a new attribute called "engine" it is a term coined by xarray (not me) But the xarray reader reads zarr which is 3 times smaller, bomb proof and can be read and written in a mulithreaded way which is near impossible with netcdf. You can also write with IO_xarray which was not actually working in the former versions. Finally it uses dask under the hood which has multiple optimisations.
I have been using the code for 2 years and I think I have saved months of computational time using it
Still not passing the tests but they are different errors
Will look into that later

Someone spotted this bracket typo in the calculation of the irradiance
Boorhin/Clyde#1
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.

2 participants