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

Split up and remove offsetting from cc.querying.getvar #148

Merged
merged 1 commit into from
Nov 13, 2019

Conversation

angus-g
Copy link
Collaborator

@angus-g angus-g commented Aug 6, 2019

See #147 for motivation behind this change. As concluded there, we can't offload everything to open_mfdataset, so restriction of the returned file set based on start/end time and overall number of files is retained.

This should be a drop-in change, although offsetting will have to be done as a separate step.

It may be worth merging #129 first (or at least in conjunction with this), because it gives the range of available times for a given variable, in the calendar/units of the files (which are used for the querying).

Closes #143, closes #136, closes #135, closes #113.

@angus-g
Copy link
Collaborator Author

angus-g commented Aug 6, 2019

@aidanheerdegen Andy has mentioned that he's willing to test this. How would we best go about making it available in the Conda environment? Can we build one out of the branch, or would it have to hit master and be tagged first?

@aidanheerdegen
Copy link
Collaborator

It is certainly possible to build it out of a branch, but if you want automatic builds and uploading to https://anaconda.org/coecms then it is master and tagged that does this. it doesn't get deployed to the conda environments automatically, so you can tag it with a -beta or similar. I can pin the unstable environment version until it is ready for release, so it doesn't get accidentally updated.

@aidanheerdegen
Copy link
Collaborator

Ok. I have pinned cosima-cookbook to 0.3.2. Merge and tag master and it won't get deployed until I remove the pin. @AndyHoggANU can test by making his own conda test env.

@AndyHoggANU
Copy link
Collaborator

OK, will do that tomorrow. But I might need more instruction on how to create that environment...

With the advances in xarray, we have ended up duplicating some of its
functionality. Paring down the data to a certain time range can be
done on the resulting DataArray, and rebasing the times is less
necessary due to more intuitive handling of CFTimes. If rebasing is
required, it's available separately.
@angus-g
Copy link
Collaborator Author

angus-g commented Aug 19, 2019

After a bit of a battle with tags and conda, I have figured out how to get a beta tag into the conda channel. @AndyHoggANU you should be able to test this (finally)!

@aidanheerdegen
Copy link
Collaborator

I have made a little repo you can clone with instructions for installing a test environment @AndyHoggANU

https://github.com/aidanheerdegen/cc-test

@AndyHoggANU
Copy link
Collaborator

Hi @angus-g and @aidanheerdegen

I managed to get Aidan's test environment working to test this version out. Note that xarray couldn't find matplotlib in this test environment, so I couldn't verify my tests by plotting the output, but I did verify that all the listed arguments worked as they should, for a few different datasets.

I think this is ready to go in, Let me know once it's there as I'll have to modify my scripts to do the offsetting separately.

@angus-g
Copy link
Collaborator Author

angus-g commented Nov 13, 2019

Any objections if I merge this? @AndyHoggANU?

@AndyHoggANU
Copy link
Collaborator

Yes please!!

@angus-g angus-g merged commit 7f536ec into COSIMA:master Nov 13, 2019
@angus-g angus-g deleted the getvar-split branch November 13, 2019 04:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants