Skip to content

Adding Field._land_value property#2451

Closed
erikvansebille wants to merge 1 commit into
mainfrom
user_defined_land_value
Closed

Adding Field._land_value property#2451
erikvansebille wants to merge 1 commit into
mainfrom
user_defined_land_value

Conversation

@erikvansebille

Copy link
Copy Markdown
Member

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 a setter on Field.land_value).

Its value is taken from the da._FillValue, and set to 0.0 if there is no _FillValue present

The Field._land_value can then be used in Interpolators that require masking of land

@erikvansebille erikvansebille Jan 6, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 VeckoTheGecko left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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)

Comment thread tests/test_field.py
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

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Comment thread tests/test_field.py
Comment on lines +171 to +173
assert field.land_value == 0.0
else:
assert field.land_value == fill_value

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess this would be a very oceanography specific part of the API?

@erikvansebille

Copy link
Copy Markdown
Member Author

Superseded by #2762

@github-project-automation github-project-automation Bot moved this from Backlog to Done in Parcels development Jul 22, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

Land as np.nan or 0 in v4?

2 participants