-
Notifications
You must be signed in to change notification settings - Fork 7
Open dataset wrapper #99
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
Conversation
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.
There are a lot of duplicate tests here. Could you instead use parameterise the tests over xr and sdfxr?
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.
Good point... since this would be quite a large refactor of the tests I'll do it in a separate branch. See #100
LiamPattinson
left a comment
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.
If the test updates will wait for a later PR, I think this should be good.
Add a lightweight wrapper for
open_datasetthat simply callsxarray.open_dataset. This is so that the user only needs to importsdf_xarrayand makes it slightly easier for new users to work with the code. Additionally added tests and updated docsSome other minor improvements made in the same branch:
drop_variablesfromopen_datatreeResolves #92
#93 merged
Wait for #93 to be merged so that theload_deckparameter can be included in theopen_datasetwrapper