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

open_dataset improvements #55

Merged
merged 20 commits into from
Mar 6, 2023
Merged

open_dataset improvements #55

merged 20 commits into from
Mar 6, 2023

Conversation

victoria-cherkas
Copy link
Collaborator

@victoria-cherkas victoria-cherkas commented Feb 27, 2023

  • Allow filtering of dataset via a variable name as argument in iconarray.open_dataset
  • Allow passing standard xarray.open_dataset arguments through iconarray.open_dataset()

@victoria-cherkas
Copy link
Collaborator Author

launch jenkins

@victoria-cherkas victoria-cherkas changed the title Draft: open_dataset improvements open_dataset improvements Feb 27, 2023
@victoria-cherkas victoria-cherkas self-assigned this Feb 27, 2023
@victoria-cherkas victoria-cherkas added the enhancement New feature or request label Feb 27, 2023
@victoria-cherkas
Copy link
Collaborator Author

launch jenkins

@victoria-cherkas
Copy link
Collaborator Author

launch jenkins

@victoria-cherkas
Copy link
Collaborator Author

C2SM/icon-vis#36 PR for icon-vis using this branch of iconarray.

https://jenkins-mch.cscs.ch/job/IconVis/job/Iconvis_PR/10/

@victoria-cherkas
Copy link
Collaborator Author

for this file grib= '/scratch/vcherkas/iconarray/data/example_data/grib/lfff00010000_edgeplots':

Elapsed time GRIB (unfiltered): ds = iconarray.open_dataset(grib)
New open_dataset(): 0:02:13
Old open_dataset(): 0:02:39

So not to worry about performance loss, it is clearly just a large file (that should probably have been cropped before uploading to the example data server).

@clairemerker
Copy link
Collaborator

Great! I think we should make it a task to reduce the test data size. We can for example crop the data to a smaller domain

@victoria-cherkas
Copy link
Collaborator Author

Great! I think we should make it a task to reduce the test data size. We can for example crop the data to a smaller domain

#57 Just added issue for it :)

Copy link
Collaborator

@clairemerker clairemerker left a comment

Choose a reason for hiding this comment

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

I made a little comment, but I really like it! I will try to implement a little test for the "variable not found" case, so maybe we can wait for this test before we merge.

iconarray/backend/grid.py Show resolved Hide resolved
iconarray/backend/grid.py Show resolved Hide resolved
@victoria-cherkas
Copy link
Collaborator Author

Ok thanks for the review @clairemerker we can wait for additional test, thanks for that

@clairemerker
Copy link
Collaborator

launch jenkins

@clairemerker
Copy link
Collaborator

launch jenkins

@clairemerker
Copy link
Collaborator

@victoria-cherkas did you already test this PR with icon-vis?

@victoria-cherkas
Copy link
Collaborator Author

@victoria-cherkas did you already test this PR with icon-vis?

Yes its passing on icon-vis :)

@victoria-cherkas victoria-cherkas merged commit d738b4c into main Mar 6, 2023
@clairemerker clairemerker deleted the open_file branch April 12, 2023 08:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants