Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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 ability to derive variables and add selected derived forcings #34
base: main
Are you sure you want to change the base?
Add ability to derive variables and add selected derived forcings #34
Changes from 50 commits
981d676
79a94db
f37161c
71afd3a
abb626b
8b1f18e
7854013
7fa90bf
48c9e3e
8de9404
ffc030c
692cdd3
c0cd875
55224f3
678ea52
9d2db07
26455bc
fbb6065
3a12f48
3ace219
a6b61b0
1814297
9dcace6
aba6757
143edb6
6aad6d7
12e0575
000ce92
0af6319
284db91
ba161d2
e3d590c
f8cae4f
8470c82
69afdd3
e17ed8b
23b119f
2ce53c7
f1e3d77
2afbb35
75797a2
31578e8
90e4cf2
717c6a5
2856c6b
98673ee
8940e82
e12e328
ecdea30
e3c0f22
e907a6d
bb9be13
49de0b3
b268f01
dbd5bfd
474a83d
1da66e2
dc7dc5e
c9e96af
47b8411
5ae772f
2c0bdf8
f1ce6d1
58d8af6
f87b954
80cf058
da0c171
f61a3b6
554f869
085aae3
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I know, that this is not introduced by this feature, but I'm not particularly fan of this very general exception, but maybe it is something that we do to get a broader explanation of the error?
Do you know why we need it, when the function itself already throw exceptions, and when we don't do anything about the exception other than reraising it?
To my mind, if a KeyError in the function is raised, we will still get the
load_and_subset_dataset
function listed in the traceback and thus know, that the error had something to do with loading the dataset.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, we shouldn't need it on
subset_dataset
. I introduced the exceptoion handling above as a convenience in the case where the user has provided a path to a input dataset that doesn't exist. The reason is that the default exception from xarray doesn't give you the path that it tried to load, so I wrapped the call so that I could raise my own exeption that includes the path. You could just put this insideload_dataset()
@ealerskans, but I think it would still be nice to wrapxr.open_dataset
etc so that the user gets the path printed when xr fails to open a datasetThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have now removed the exception wrapping for
subset_dataset
andderive_variables
. But I have kept it wrapped around theload_input_dataset
function call since I didn't want to have to pass bothdataset_name
andpath
to theload_input_dataset
function. Let me know what you think.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me:)