Add support for reading pixel_quality ancillary variables, FCI reader no longer logs warnings

[gerritholl]

Add support for reading pixel quality, closing #1171

Read pixel quality ancillary variables. This is done with a small hack, because the data variables have the same name in each group, so Satpy doesn't know where to look when simply told to look for pixel_quality. This may change later when dynamic dataset ids are introduced (see #1088) but so far this is not supported. Therefore, when reading the parent variable, the filehandler rewrites the ancillary_variables variable attribute to prepend the channel name, such that when the same filehandler is called a little later for the variable name, it knows where to find it. See also the discussion in #1171.

• Closes Add reading of pixel_quality variable to FCI FDHSI reader #1171
• Tests added
• Tests passed
• Passes flake8 satpy
• Fully documented

[coveralls]

Coverage increased (+0.06%) to 89.678% when pulling 9c0adbc on gerritholl:fci-pixel-quality into ef24f29 on pytroll:master.

[codecov]

Codecov Report

Merging #1177 into master will increase coverage by 0.05%.
The diff coverage is 98.18%.

@@            Coverage Diff             @@
##           master    #1177      +/-   ##
==========================================
+ Coverage   89.61%   89.67%   +0.05%     
==========================================
  Files         200      200              
  Lines       29523    29722     +199     
==========================================
+ Hits        26458    26654     +196     
- Misses       3065     3068       +3     

Impacted Files	Coverage Δ
satpy/readers/fci_l1c_fdhsi.py	98.06% <94.11%> (+1.68%)	⬆️
satpy/tests/reader_tests/test_fci_l1c_fdhsi.py	100.00% <100.00%> (ø)
satpy/readers/avhrr_l1b_gaclac.py	93.33% <0.00%> (-1.34%)	⬇️
satpy/readers/hrit_jma.py	97.94% <0.00%> (-1.22%)	⬇️
satpy/writers/cf_writer.py	93.77% <0.00%> (-0.03%)	⬇️
satpy/writers/geotiff.py	90.74% <0.00%> (ø)
satpy/readers/netcdf_utils.py	100.00% <0.00%> (ø)
satpy/readers/seviri_l1b_native.py	71.96% <0.00%> (ø)
satpy/tests/reader_tests/test_ahi_hrit.py	100.00% <0.00%> (ø)
satpy/tests/reader_tests/test_seviri_l1b_native.py	95.16% <0.00%> (ø)
... and 9 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ef24f29...9c0adbc. Read the comment docs.

[mraspaud]

Thanks for adding the pixel quality datasets, it'll be crucial for the validation of the data when the satellite launches. I just have a couple of style suggestions, otherwise LGTM.

[djhoese]

This looks overall good to me. I had one comment inline about handling the ancillary_variables attribute, but one other point that I think @mraspaud made on slack over somewhere: What do you think about using available_datasets in the file handler to define these new datasets instead of listing them in the YAML?

I am also not a huge fan of sensor: being defined for each dataset. I feel like that should come from the file handler.

[gerritholl]

Sorry, didn't notice until now you'd left comments here.

There seem to be many readers defining sensor: in the yaml file, I just followed the precedent there.

What is the advantage of available_datasets() over defining the format in the yaml file if every file must have the same content? We don't need to read the file to know what should be in it, and if any of those datasets are not in it, it is a bug. Isn't the whole point of the yaml file to keep the definition of what's in the file out of the source code?

[djhoese]

Isn't the whole point of the yaml file to keep the definition of what's in the file out of the source code?

Yes. You could also see it as the YAML helps us define things that aren't known from reading the file. For example, the wavelengths of a band are normally in the YAML because they aren't defined in the data files. I think defining the datasets you have in YAML is fine since you've already done it, but available_datasets could be nice to automatically support future datasets if they are added to the files (assuming resolution and other metadata can be determined from the files). With FCI having so many files per time step though it is probably better to keep it in the YAML or we could end up with a recursion limit problem of calling available_datasets for every file handler.