Add LI L2 reader

[seenno]

Add LI L2 reader and remove the old code as it's obsolete.
The reader handles both, LI L2 point products (LI-LE, LI-LEF, LI-LGR, LI-LFL) and LI L2 accumulated products (LI-AF, LI-AFR, LI-AFA). For the accumulated products, the reader supports the output as a gridded product. This can be activated by adding the reader kwarg with_area_definition=True.

• Closes Update li_l2-reader to read MTG LI L2 test data #2213
• Tests added
• Fully documented
• Add your name to AUTHORS.md if not there already

[ameraner]

The tests are failing with

self = <[RecursionError('maximum recursion depth exceeded') raised in repr()] DataArray object at 0x7feb90f7dc40>
data = dask.array<broadcast_to, shape=(123,), dtype=datetime64[ns], chunksize=(123,), chunktype=numpy.ndarray>
coords = Coordinates:
    *empty*, dims = ('scalar',), name = None, attrs = {}
indexes = Indexes:
    *empty*, fastpath = False

    def __init__(
        self,
        data: Any = dtypes.NA,
        coords: Sequence[Sequence[Any] | pd.Index | DataArray]
        | Mapping[Any, Any]
        | None = None,
        dims: Hashable | Sequence[Hashable] | None = None,
        name: Hashable = None,
        attrs: Mapping = None,
        # internal parameters
        indexes: dict[Hashable, Index] = None,
        fastpath: bool = False,
    ) -> None:
        if fastpath:
            variable = data
            assert dims is None
            assert attrs is None
            assert indexes is not None
        else:
            # TODO: (benbovy - explicit indexes) remove
            # once it becomes part of the public interface
            if indexes is not None:
>               raise ValueError("Providing explicit indexes is not supported yet")
E               ValueError: Providing explicit indexes is not supported yet

/usr/share/miniconda3/envs/test-environment/lib/python3.9/site-packages/xarray/core/dataarray.py:395: ValueError

at line https://github.com/pytroll/satpy/pull/2271/files#diff-3b2bff08b4001ec6f72cca67791cc4322b38e0db97e68f2109791093e56e6052R595
We actually need to define the indexes for the reader to work as intended...

It seems a regression of xarray, since the tests are passing e.g. with xarray-2022.3.0, but failing with 2022.11.0 and the current xarray master.
Any idea? @djhoese is this related to the RecursionError issues we saw for the ancillary_variables recently?

[djhoese]

No this isn't related to my stuff at all. Did this work in the past? Why do you need to provide the indexes from the base DataArray?

[mraspaud]

Some preliminary comments. Also, CodeScene makes good points

[mraspaud]

don't we already have this implemented somewhere ? In pyresample maybe?

[ameraner]

The code was inspired by this code snippet here https://github.com/pytroll/pyresample/blob/952cec6ee4bce55bb70f2c1237401356645bc3f6/pyresample/geometry.py#L2434-L2441, that is encapsulated in get_lonlats of AreaDefinition. As in the end it's only a simple map_blocks call, I would propose to leave it like this, if you agree.

[codecov]

Codecov Report

Merging #2271 (fed60b7) into main (c81e02e) will increase coverage by 0.24%.
The diff coverage is 99.76%.

@@            Coverage Diff             @@
##             main    #2271      +/-   ##
==========================================
+ Coverage   94.32%   94.56%   +0.24%     
==========================================
  Files         308      313       +5     
  Lines       46168    47425    +1257     
==========================================
+ Hits        43546    44849    +1303     
+ Misses       2622     2576      -46     

Flag	Coverage Δ
behaviourtests	4.51% <0.00%> (-0.10%)	⬇️
unittests	95.20% <99.76%> (+0.24%)	⬆️

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

Impacted Files	Coverage Δ
satpy/tests/reader_tests/_li_test_utils.py	99.23% <99.23%> (ø)
satpy/tests/reader_tests/test_li_l2_nc.py	99.75% <99.75%> (ø)
satpy/readers/li_base_nc.py	100.00% <100.00%> (ø)
satpy/readers/li_l2_nc.py	100.00% <100.00%> (ø)
satpy/readers/netcdf_utils.py	98.32% <0.00%> (-1.68%)	⬇️
satpy/tests/test_resample.py	88.90% <0.00%> (-0.37%)	⬇️
satpy/readers/yaml_reader.py	97.49% <0.00%> (-0.01%)	⬇️
satpy/tests/test_utils.py	100.00% <0.00%> (ø)
satpy/readers/seviri_base.py	100.00% <0.00%> (ø)
satpy/tests/reader_tests/test_fci_l1c_nc.py	100.00% <0.00%> (ø)
... and 18 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

[ameraner]

Sorry @djhoese, I just noticed that the code formatting in my comment swallowed the first line (edited now), which somehow points to a RecursionError.. that's why I was asking about the RecursionErrors we saw lately.

But the issue seems actually to be in the passing of the indexes. The reader actually works without it, so we just removed it (note that this code has been written by external contractors, so we are not fully aware of all the code details).

More info on the indexes if you're interested, from my investigation:
In old-ish versions of xarray (<=2022.03), xr.DataArray was accepting indexes in the init, but I think it was not actually using it unless when passing fastpath=True (see this bit from 2022.03 https://github.com/pydata/xarray/blob/v2022.03.0/xarray/core/dataarray.py#L404-L416, the docstring doesn't mention this..). From 2022.06.0rc0 on, they implemented the ValueError check referenced above, I suppose to avoid this silent ignoring.. However, it seems that xarray devs are picking up this again in the last weeks, see pydata/xarray#7214.

[coveralls]

Coverage increased (+0.2%) to 95.149% when pulling fed60b7 on seenno:add_lil2_reader into c81e02e on pytroll:main.

[pnuu]

LGTM

[sjoro]

a small request to maybe define a new constant in order to avoid hard coding 5568 in various places in the code.

[sjoro]

only one comment to clarify, otherwise looks good to me! thanks for your efforts on this reader @seenno and @ameraner !

[ameraner]

So we added some new tests to improve coverage (now we're at 100% for both the reader files 🙂). Also most CodeScene comments were addressed, the few remaining ones complain about the tests setup file, that contains the mocked files structures... I don't think it makes sense to improve this further.
So from our side this is ready now.

[sjoro]

LGTM!

[seenno]

Thank you everyone who contributed, either as a reviewer or helping me to understand Satpy and its coding style :)