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

194 write method added to NetCDFFieldList method #195

Merged

Conversation

EddyCMWF
Copy link
Contributor

write method added to NetCDFFieldList method, simply redirect to the to_netCDF method.

@EddyCMWF EddyCMWF linked an issue Sep 19, 2023 that may be closed by this pull request
@sandorkertesz
Copy link
Collaborator

Please note that the to_netcdf method is not properly implemented, since it is saving the original netcdf data and ignoring the actual fieldlist structure.

@EddyCMWF
Copy link
Contributor Author

Please note that the to_netcdf method is not properly implemented, since it is saving the original netcdf data and ignoring the actual fieldlist structure.

I think this is what we want, right? If we are just saving the object we want the original file where possible with minimal modification from EK.

Actually I was a little unsure whether we should even be using xarray for opening and then writing, but I think that change can be part of another issue and PR.

@sandorkertesz
Copy link
Collaborator

sandorkertesz commented Sep 19, 2023

Please note that the to_netcdf method is not properly implemented, since it is saving the original netcdf data and ignoring the actual fieldlist structure.

I think this is what we want, right? If we are just saving the object we want the original file where possible with minimal modification from EK.

Actually I was a little unsure whether we should even be using xarray for opening and then writing, but I think that change can be part of another issue and PR.

Please consider these examples:

# ds is a fieldlist with e.g. 100 fields
ds = earthkit.data.from_source("file", "my_data.nc")
f = ds[2:10]
f.save("my_file.nc")
# ds1 is a fieldlist with 10 fields
ds1 = earthkit.data.from_source("file", "my_data_1.nc")
# ds2 is a fieldlist with 20 fields
ds2 = earthkit.data.from_source("file", "my_data_2.nc")
# concatenation
ds = ds1 + ds2
f = ds[8:12]
f.save("my_file.nc")

@EddyCMWF
Copy link
Contributor Author

Please note that the to_netcdf method is not properly implemented, since it is saving the original netcdf data and ignoring the actual fieldlist structure.

I think this is what we want, right? If we are just saving the object we want the original file where possible with minimal modification from EK.
Actually I was a little unsure whether we should even be using xarray for opening and then writing, but I think that change can be part of another issue and PR.

Please consider these examples:

# ds is a fieldlist with e.g. 100 fields
ds = earthkit.data.from_source("file", "my_data.nc")
f = ds[2:10]
f.save("my_file.nc")
# ds1 is a fieldlist with 10 fields
ds1 = earthkit.data.from_source("file", "my_data_1.nc")
# ds2 is a fieldlist with 20 fields
ds2 = earthkit.data.from_source("file", "my_data_2.nc")
# concatenation
ds = ds1 + ds2
f = ds[8:12]
f.save("my_file.nc")

I added a not implemented error to the NetCDFMaskedFieldList as this was producing an error.

The MultiFile looks to work if the 2 data objects are concatable.

@sandorkertesz
Copy link
Collaborator

sandorkertesz commented Sep 19, 2023

The MultiFile looks to work if the 2 data objects are concatable.

Should be tested. But please consider this code:

# ds1 is a fieldlist with 10 fields
ds1 = earthkit.data.from_source("file", "my_data_1.nc")
# ds2 is a fieldlist with 20 fields
ds2 = earthkit.data.from_source("file", "my_data_2.nc")

f1 = ds1[2:5]
f2 = ds2[8:14]
f = f1 + f2
f.save("my_file.nc")

@EddyCMWF
Copy link
Contributor Author

EddyCMWF commented Sep 19, 2023

The MultiFile looks to work if the 2 data objects are concatable.

Should be tested. But please consider this code:

# ds1 is a fieldlist with 10 fields
ds1 = earthkit.data.from_source("file", "my_data_1.nc")
# ds2 is a fieldlist with 20 fields
ds2 = earthkit.data.from_source("file", "my_data_2.nc")

f1 = ds1[2:5]
f2 = ds2[8:14]
f = f1 + f2
f.save("my_file.nc")

But this is a problem with the to_xarray method of the NetCDFMaskedFieldList object, which I don't think is the responsibility of this issue and PR. I have added another not implemented exception for when the objects do not have a path attribute.

Just to be clear, the following currently fails, and I believe is outside the scope of this PR:

# ds1 is a fieldlist with 10 fields
ds1 = earthkit.data.from_source("file", "my_data_1.nc")
# ds2 is a fieldlist with 20 fields
ds2 = earthkit.data.from_source("file", "my_data_2.nc")
 
f1 = ds1[2:5]
f2 = ds2[8:14]
f = f1 + f2
# This will fail:
f.to_xarray()
# This will also fail:
f.to_netcdf("my_file.nc")

@EddyCMWF
Copy link
Contributor Author

I have moved the "Not implemented" Exception to the correct layer, i.e. on the to_xarray method of the NetCDFMaskFieldList

@sandorkertesz sandorkertesz merged commit 683c80e into develop Sep 25, 2023
11 checks passed
@EddyCMWF
Copy link
Contributor Author

hey @sandorkertesz, thanks for finishing this off :)

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.

NetCDFFieldList does not have a write method
2 participants