-
Notifications
You must be signed in to change notification settings - Fork 143
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
Test correct keys in dimensions dictionary #545
Conversation
Also adding unit test
Instead using new `filebuffername` variable
parcels/field.py
Outdated
@@ -55,7 +55,8 @@ class Field(object): | |||
def __init__(self, name, data, lon=None, lat=None, depth=None, time=None, grid=None, mesh='flat', | |||
fieldtype=None, transpose=False, vmin=None, vmax=None, time_origin=None, | |||
interp_method='linear', allow_time_extrapolation=None, time_periodic=False, **kwargs): | |||
self.name = name | |||
self.name = list(name)[0] if isinstance(name, dict) else name | |||
self.filebuffername = name[list(name)[0]] if isinstance(name, dict) else name |
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.
It's quite hard to follow. Why is name
a dict?
If you make name
a tuple, you can do
if not isinstance(name, tuple):
self.name = name
self.filebuffername = name
else:
self.name, self.filebuffername = name
or as you did
self.name = name[0] if isinstance(name, tuple) else name
self.filebuffername = name[1] if isinstance(name, tuple) else name
parcels/field.py
Outdated
@@ -133,7 +134,7 @@ def from_netcdf(cls, filenames, variable, dimensions, indices=None, grid=None, | |||
filenames can be a list [files] | |||
or a dictionary {dim:[files]} (if lon, lat, depth and/or data not stored in same files as data) | |||
time values are in filenames[data] | |||
:param variable: Name of the field to create. Note that this has to be a string | |||
:param variable: Dictionary mapping name of the field to create to variable in file. |
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.
So here I would put a tuple. You will never have more than one key, right?
parcels/fieldset.py
Outdated
@@ -34,6 +34,12 @@ def __init__(self, U, V, fields=None): | |||
|
|||
self.compute_on_defer = None | |||
|
|||
@classmethod |
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.
This should be a static method
This PR tests that the
dimensions
dictionary has no other keys thanlon
,lat
,depth
andtime
. Since other keys are not used, this prevents errors where users use the wrong key for e.g.depth
and inadvertently use in 2D modeFurthermore, this PR also cleans up the code by removing the
dimensions['data']
entry for the name of the variable in the filebuffer