Skip to content

Make sure FCI L2 reader returns cloud type and other categorical data as int#3339

Open
gerritholl wants to merge 2 commits intopytroll:mainfrom
gerritholl:bug-fci-cloud-type
Open

Make sure FCI L2 reader returns cloud type and other categorical data as int#3339
gerritholl wants to merge 2 commits intopytroll:mainfrom
gerritholl:bug-fci-cloud-type

Conversation

@gerritholl
Copy link
Member

@gerritholl gerritholl commented Feb 17, 2026

Make sure that the FCI L2 reader returns categorical data, such as cloud type, cloud phase, cloud mask, pixel quality etc. as an exact dtype, de facto an integer dtype (in the source data those are enum).

This change may break backwards compatibility for users expecting a cloud mask or a cloud type as a float.

Add a test case reproducing
pytroll#3338, where cloud type becomes a
float.

Test fails.
@gerritholl gerritholl added the backwards-incompatibility Causes backwards incompatibility or introduces a deprecation label Feb 18, 2026
In the FCI L2 data, do not convert categorical data types (quality,
cloud type, cloud phase, cloud mask, etc.) into float.

Fixes pytroll#3338
@gerritholl gerritholl marked this pull request as ready for review February 18, 2026 08:32
@codecov
Copy link

codecov bot commented Feb 18, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 96.34%. Comparing base (21b03f6) to head (752fafd).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #3339   +/-   ##
=======================================
  Coverage   96.34%   96.34%           
=======================================
  Files         463      463           
  Lines       58959    58962    +3     
=======================================
+ Hits        56802    56805    +3     
  Misses       2157     2157           
Flag Coverage Δ
behaviourtests 3.59% <0.00%> (-0.01%) ⬇️
unittests 96.43% <100.00%> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@coveralls
Copy link

Pull Request Test Coverage Report for Build 22132283193

Details

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 96.421%

Totals Coverage Status
Change from base Build 21982018474: 0.0%
Covered Lines: 56685
Relevant Lines: 58789

💛 - Coveralls

@strandgren
Copy link
Collaborator

With the proposed fix you effectively disable the fill_value functionality for the integer data. In that case we should probably remove all fill_value=-127 entries in the fci_l2_nc.yaml file as well in order to be consistent and avoid confusion. That means that it would also be removed from the xarray.DataArray attribute, but since it's not really correct I don't see that as a problem?

Another solution would be to treat float and integer data independently in _mask_data (https://github.com/gerritholl/satpy/blob/bug-fci-cloud-type/satpy/readers/fci_l2_nc.py#L157). If done properly we could then set the requested fill values to NaN for float data and to the corresponding fill value for different integer data (-127, 255, -32767, 65535, etc). I have read somewhere about determining the relevant integer fill-value for a given data type in satpy, but I can't recall where. On the other hand I don't see the added value of such a solution, since I don't think we currently want to convert any integer values to a given (different) fill-value (which we also don't do now).

Hence, I'd vote for the first alternative (which you have started), but also cleaning up the corresponding fill_value=-127 entries in the yaml-file.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backwards-incompatibility Causes backwards incompatibility or introduces a deprecation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

FCI L2 reader returns categorical data as float

3 participants