Add new mask datasets for olci l2 and refactor a bit

[mraspaud]

This PR add the cloud_mask and ocnn_mask datasets to the olci l2 reader. This superseeds and closes #741.

Some refactoring has been done, as well as passing the engine through to xr.open_dataset.

• Closes Add cloud_mask as a dataset for olci l2 data #741
• Tests added
• Fully documented

[codecov]

Codecov Report

Attention: Patch coverage is 99.50249% with 1 line in your changes missing coverage. Please review.

Project coverage is 93.92%. Comparing base (20434bf) to head (67489e0).
Report is 4230 commits behind head on main.

Files with missing lines	Patch %	Lines
satpy/readers/olci_nc.py	98.07%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1767      +/-   ##
==========================================
+ Coverage   93.89%   93.92%   +0.02%     
==========================================
  Files         283      283              
  Lines       42589    42707     +118     
==========================================
+ Hits        39991    40111     +120     
+ Misses       2598     2596       -2     

Flag	Coverage Δ
behaviourtests	4.69% <0.00%> (-0.02%)	⬇️
unittests	94.47% <99.50%> (+0.02%)	⬆️

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.

[coveralls]

Coverage increased (+0.02%) to 94.418% when pulling 67489e0 on mraspaud:feature-olci-l2-datasets-take-2 into 20434bf on pytroll:main.

[djhoese]

Just putting this here to sanity check, dask distributed can handle static methods right?

[mraspaud]

no clue. I'll make it a function just in case.

[djhoese]

Besides my NotImplementedError comment, this looks good to me.

[johannesocean]

@mraspaud You could also change the mask dataset to:

  mask:
    masked_items: [ "INVALID", "SNOW_ICE", "INLAND_WATER", "SUSPECT",
                    "AC_FAIL", "CLOUD", "HISOLZEN", "COASTLINE",
                    "CLOUD_MARGIN", "CLOUD_AMBIGUOUS", "LOWRW", "LAND" ]

removing "OCNN_FAIL" and adding "COASTLINE".

[johannesocean]

Beside the mask, I would also like to add new datasets: w_aer and iwv. However, if you prefer me to make a separate PR (?), I´ll do that after this PR is merged.

file_types:
  esa_l2_iwv:
    file_reader: !!python/name:satpy.readers.olci_nc.NCOLCI2
    file_patterns: ['{mission_id:3s}_OL_2_{datatype_id:_<6s}_{start_time:%Y%m%dT%H%M%S}_{end_time:%Y%m%dT%H%M%S}_{creation_time:%Y%m%dT%H%M%S}_{duration:4d}_{cycle:3d}_{relative_orbit:3d}_{frame:4d}_{centre:3s}_{mode:1s}_{timeliness:2s}_{collection:3s}.SEN3/iwv.nc']
  esa_l2_w_aer:
    file_reader: !!python/name:satpy.readers.olci_nc.NCOLCI2
    file_patterns: ['{mission_id:3s}_OL_2_{datatype_id:_<6s}_{start_time:%Y%m%dT%H%M%S}_{end_time:%Y%m%dT%H%M%S}_{creation_time:%Y%m%dT%H%M%S}_{duration:4d}_{cycle:3d}_{relative_orbit:3d}_{frame:4d}_{centre:3s}_{mode:1s}_{timeliness:2s}_{collection:3s}.SEN3/w_aer.nc']

datasets:
  iwv:
    name: iwv
    sensor: olci
    resolution: 300
    calibration:
      reflectance:
        standard_name: integrated_water_vapour_column
        units: "lg(re m-l)"
    coordinates: [longitude, latitude]
    file_type: esa_l2_iwv
    nc_key: IWV

  w_aer:
    name: w_aer
    sensor: olci
    resolution: 300
    calibration:
      reflectance:
        standard_name: aerosol_optical_thickness
        units: "lg(re g.m-3)"
    coordinates: [longitude, latitude]
    file_type: esa_l2_w_aer
    nc_key: T865

[mraspaud]

@JohannesSMHI ok, about the mask: could you explain the thinking behind the change?

[ghost]

DeepCode's analysis on #d60539 found:

• ℹ️ 1 minor issue. 👇

Top issues

Description	Example fixes
In Python 2, defining only __eq__ but not __ne__ will result in an error if objects are compared with inequality. Occurrences: olci_nc.py:82	🔧 Example fixes

👉 View analysis in DeepCode’s Dashboard | Configure the bot

👉 The DeepCode service and API will be deprecated in August, 2021. Here is the information how to migrate. Thank you for using DeepCode 🙏 ❤️ !

If you are using our plugins, you might be interested in their successors: Snyk's JetBrains plugin and Snyk's VS Code plugin.

[mraspaud]

@JohannesSMHI I added the datasets that you mentioned above. However, looking at the standard names, I realize they are not defined in the cf convention document: http://cfconventions.org/Data/cf-standard-names/77/build/cf-standard-name-table.html
What is the use for standard names here? We should be careful how we choose them in this case, and make sure we are consistent across the different readers.

[johannesocean]

@JohannesSMHI ok, about the mask: could you explain the thinking behind the change?

OCNN_FAIL seems to be over sensitive during summertime in the Baltic Sea. I now realize that it might be a bit too regional of a problem, and that we may not want to push this particular change to the main branch?

When it comes to COASTLINE I would definitely add that to the main branch since the coastline almost always gives us bad data.

[johannesocean]

@JohannesSMHI I added the datasets that you mentioned above. However, looking at the standard names, I realize they are not defined in the cf convention document: http://cfconventions.org/Data/cf-standard-names/77/build/cf-standard-name-table.html
What is the use for standard names here? We should be careful how we choose them in this case, and make sure we are consistent across the different readers.

Do we even need to specify the standard_name? I simply added it using the descripting of the product (unaware the cf convention document)

[mraspaud]

Yes, standard names allows use to make sure we provide interoperable datasets

[JonDHo]

Hi. This PR looks super close to being resolved and it would be a great addition, but is there any chance of also adding PAR to the list along with the other additions in above (#1767 (comment)). That would then complete the full list of olci_l2 measurements.

[mraspaud]

I have a feeling this has been superseeded by #2504, so I'm thinking of closing this. Anyone against?