-
Notifications
You must be signed in to change notification settings - Fork 3
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
add test for E3SM data #27
base: main
Are you sure you want to change the base?
Conversation
@chengzhuzhang the test looks great, thank you for adding it. While @tomvothecoder may comment on your points, I have a few thoughts: |
Hey @lee1043 thanks a lot, I made an update to reflect the input data source, and added you to author list. |
@chengzhuzhang I am not serious about having my name there, but thanks for listing me on the notebook. I don't mind merging small PRs multiple times or big PRs fewer times. Please feel free to merge it whenever you feel comfortable. Many thanks for working on this. |
I agree, having another demo with a different model format sounds useful.
I think the robust implementation is having a separate This method has been implemented in xCDAT/xcdat#107 and can be called like so:
Thanks for the catch, I will open an issue to fix this typo.
Sounds good! Note: Rename as |
@chengzhuzhang this PR looks ready to be merged, would you be okay with that? |
@tomvothecoder @lee1043, hey, thanks both for your input. I will think more about the implementation of |
sorry for the food of committing messages. I'm less familiar with committing changes in JupyterNotebook, and the committing history may be messed up. I will have a new PR later and will discard this one... |
@chengzhuzhang No worries, you can squash the commits if you want. |
This is an initial PR for testing xcdat general functionalities. It is a copy of @lee1043 's demo, but apply to E3SM data. We can re-organize later to remove redundancy, but at this point, it just help to visualize the test results with another model format.
centering time
for climo calculation, maybe this step should be generalized and integrate intodecode_time
.data_var
should replacekeep_var
in the docs?