Skip to content

Ensure that _FillValue and statistical attributes have the same data type as the correspondind layer#213

Open
gshiroma wants to merge 24 commits intoisce-framework:developfrom
gshiroma:fix_fill_value_stats_dtype
Open

Ensure that _FillValue and statistical attributes have the same data type as the correspondind layer#213
gshiroma wants to merge 24 commits intoisce-framework:developfrom
gshiroma:fix_fill_value_stats_dtype

Conversation

@gshiroma
Copy link
Contributor

@gshiroma gshiroma commented Feb 9, 2026

This PR ensures that _FillValue and statistical attributes have the same data type as the corresponding layer.

gshiroma and others added 24 commits July 3, 2025 11:30
@gshiroma gshiroma requested a review from hfattahi February 9, 2026 19:58
@bhawkins
Copy link
Contributor

bhawkins commented Feb 9, 2026

This PR ensures that _FillValue and statistical attributes have the same data type as the corresponding layer.

That makes sense for _FillValue. Do you always want that for stats, though? If you compute the mean of an integer-valued image in floating point it might be preferable to report it that way.

@gshiroma
Copy link
Contributor Author

gshiroma commented Feb 9, 2026

This PR ensures that _FillValue and statistical attributes have the same data type as the corresponding layer.

That makes sense for _FillValue. Do you always want that for stats, though? If you compute the mean of an integer-valued image in floating point it might be preferable to report it that way.

Thank you for your comment, @bhawkins. We do not compute statistics for the mask layer. Also, note that this code is implemented in _get_attribute_dict(), where the default value for the to_data_format_function parameter is lambda x: x. This means that, by default, the data type of _FillValue and the statistical values is not modified. This function is (indirectly) called by save_dataset(), which is only used by GCOV and by the code that geocodes RSLC LUTs. So, I don’t see this concern occurring in the current products. But it's a good point. Even if that happened, there could be cases in which it could be preferable to compute statistics of integer datasets in float or integer.

For context, I added this feature after noticing that some statistical values were being saved as float64 for float32 datasets, and this PR fixes that problem.

Copy link
Contributor

@bhawkins bhawkins left a comment

Choose a reason for hiding this comment

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

Makes sense.

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