Adding Field._land_value property #2451
Conversation
There was a problem hiding this comment.
Need to explicitly set the _FillValue on these fields below to avoid errors on the test_fieldset_from_copernicusmarine_no_logs unit test in test_fieldset.py; because without these _FillValues the Field creation throws a log message
VeckoTheGecko
left a comment
VeckoTheGecko
left a comment
There was a problem hiding this comment.
I think that there are other ways to implement this behaviour without having to work with _FillValue or assign a land value (which have drawbacks as mentioned in my other comments) . Perhaps a good place to start would be to create a dataset that has U V and something like salinity, and see from a user POV how they can get intended behaviour without modifying Parcels internals? (e.g., via xr.DataArray.fillna)
| def test_field_land_value(fill_value): | ||
| ds = datasets_structured["ds_2d_left"].copy() | ||
| if fill_value is not None: | ||
| ds["data_g"].attrs["_FillValue"] = fill_value |
There was a problem hiding this comment.
I think here we're kind of overloading and conflicting with how xarray ingests this
https://docs.xarray.dev/en/stable/user-guide/io.html#scaling-and-type-conversions
i.e., the user merely opening a dataset with decoding enabled might cause the _FillValue to be interpretted as np.nan, and then removed from the xarray dataset metadata.
I think a more informative test would be to look at reading in a dataset from disk rather than using this example dataset
| assert field.land_value == 0.0 | ||
| else: | ||
| assert field.land_value == fill_value |
There was a problem hiding this comment.
I guess this would be a very oceanography specific part of the API?
This PR fixes #2393 by giving users control over the value in their Fields that represents land points. This value is stored in
Field._land_value(which can be modified by asetteronField.land_value).Its value is taken from the
da._FillValue, and set to0.0if there is no_FillValuepresentThe
Field._land_valuecan then be used in Interpolators that require masking of land