Skip to content
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

fix: handle admp valid start index for retention period #138

Merged

Conversation

renaudjester
Copy link
Collaborator

We would set the minimun to the admp_valid_start_date whereas some datasets have values and hence we should return only the valid values for the time values. See an example:

https://stac.marine.copernicus.eu/metadata/OCEANCOLOUR_ATL_BGC_L4_NRT_009_116/cmems_obs-oc_atl_bgc-pp_nrt_l4-multi-1km_P1M_202311/dataset.stac.json

@renaudjester renaudjester requested a review from uriii3 September 18, 2024 07:19
@uriii3
Copy link
Collaborator

uriii3 commented Sep 18, 2024

When calling the function, the admp_valid_start_index and the admp_valid_start_date will always be there? What happens if the code doesn't find it?
It is weird because in the example you posted (this one), the admp_valid_start_index is 9, so we would start to count in the "values" from there, no? Which doesn't make that sense to me as to why there would be the other values listed in the metadata. Ex: in the metadata above:
"values": [ "2023-09-01T00:00:00Z", "2023-10-01T00:00:00Z", "2023-11-01T00:00:00Z", "2023-12-01T00:00:00Z", "2024-01-01T00:00:00Z", "2024-02-01T00:00:00Z", "2024-03-01T00:00:00Z", "2024-04-01T00:00:00Z", "2024-05-01T00:00:00Z", "2024-06-01T00:00:00Z", "2024-07-01T00:00:00Z", "2024-08-01T00:00:00Z" ]
and then "admp_valid_start_index": 9,
Why would you have all those values just to point out that the minimum (valid) is the ninth?

chunking_length = dimension_metadata.get("chunkLen")
if isinstance(chunking_length, dict):
chunking_length = chunking_length.get(variable_id)

coordinate = cls(
coordinate_id=dimension,
units=dimension_metadata.get("units") or "",
minimum_value=minimum_value, # type: ignore
minimum_value=minimum_value or coordinates_info.get("min"), # type: ignore
Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't really understand this: if there is a minimum_value assigned (not None) it takes it, else it takes coordinates_info.get("min")? What happens if there is no coordinates_info.get("min")? It throws an error, no?

Copy link
Collaborator

Choose a reason for hiding this comment

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

It wouldn't enter the if and then it would have, at max, a None as minimum value.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It wouldn't throw an error only coordinates_info.get("min") whatever the value is

@renaudjester renaudjester merged commit 0367964 into copernicusmarine-toolbox-v2 Sep 18, 2024
2 checks passed
@renaudjester renaudjester deleted the handle-values-with-retention-period branch September 18, 2024 12:38
renaudjester added a commit that referenced this pull request Oct 28, 2024
Handle values for retention dates as some datasets don't have min and max but only a list of values that should be updated
renaudjester added a commit that referenced this pull request Oct 28, 2024
Handle values for retention dates as some datasets don't have min and max but only a list of values that should be updated
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants