Saving an image with NinjoTIFFWriter is broken in satpy v.0.20.0

[avalentino]

Describe the bug
It seems that saving an image with NinjoTIFFWriter is broken in satpy v.0.20.0.
The failure happens both with pyninjotiff v0.2 and with the recent pyninjotiff v0.3 even if the error seems to be a little bit different.

To Reproduce

$ python3.8 setup.py test

Expected behavior
The test suite is executed without errors

Actual results
The execution of the test suite fails with one error:

Test with pyninjotiff v0.3.0

======================================================================
ERROR: test_image (satpy.tests.writer_tests.test_ninjotiff.TestNinjoTIFFWriter)
Test saving an image.
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/usr/lib/python3.8/unittest/mock.py", line 1778, in _inner
    return f(*args, **kw)
  File "/usr/lib/python3.8/unittest/mock.py", line 1348, in patched
    return func(*newargs, **newkeywargs)
  File "/home/antonio/debian/git/build-area/satpy-0.20.0/satpy/tests/writer_tests/test_ninjotiff.py", line 75, in test_image
    ret = ntw.save_image(img, filename='bla.tif', compute=False)
  File "/home/antonio/debian/git/build-area/satpy-0.20.0/satpy/writers/ninjotiff.py", line 155, in save_image
    return nt.save(img, filename, data_is_scaled_01=True, compute=compute, **kwargs)
  File "/usr/lib/python3/dist-packages/pyninjotiff/ninjotiff.py", line 541, in save
    area_def = img.data.area
  File "/usr/lib/python3/dist-packages/xarray/core/common.py", line 232, in __getattr__
    raise AttributeError(
AttributeError: 'DataArray' object has no attribute 'area'

----------------------------------------------------------------------
Ran 684 tests in 109.051s

FAILED (errors=1, skipped=6)
Test failed: <unittest.runner.TextTestResult run=684 errors=1 failures=0>
[[25. 25. 25. 25. 25. 25. 25. 25. 25. 25.]
 [25. 25. 25. 25. 25. 25. 25. 25. 25. 25.]
 [25. 25. 25. 25. 25. 25. 25. 25. 25. 25.]
 [50. 50. 50. 50. 50. 50. 50. 50. 50. 50.]
 [75. 75. 75. 75. 75. 75. 75. 75. 75. 75.]]

Test with pyninjotiff v0.2.0

======================================================================
ERROR: test_image (satpy.tests.writer_tests.test_ninjotiff.TestNinjoTIFFWriter)
Test saving an image.
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/usr/lib/python3.8/unittest/mock.py", line 1778, in _inner
    return f(*args, **kw)
  File "/usr/lib/python3.8/unittest/mock.py", line 1348, in patched
    return func(*newargs, **newkeywargs)
  File "/build/satpy-0.20.0/satpy/tests/writer_tests/test_ninjotiff.py", line 75, in test_image
    ret = ntw.save_image(img, filename='bla.tif', compute=False)
  File "/build/satpy-0.20.0/satpy/writers/ninjotiff.py", line 155, in save_image
    return nt.save(img, filename, data_is_scaled_01=True, compute=compute, **kwargs)
  File "/usr/lib/python3/dist-packages/pyninjotiff/ninjotiff.py", line 559, in save
    data, scale, offset, fill_value = _finalize(img,
  File "/usr/lib/python3/dist-packages/pyninjotiff/ninjotiff.py", line 441, in _finalize
    data[mask] = fill_value
TypeError: 'numpy.uint8' object does not support item assignment

----------------------------------------------------------------------
Ran 684 tests in 95.166s

FAILED (errors=1, skipped=6)
Test failed: <unittest.runner.TextTestResult run=684 errors=1 failures=0>
[[25. 25. 25. 25. 25. 25. 25. 25. 25. 25.]
 [25. 25. 25. 25. 25. 25. 25. 25. 25. 25.]
 [25. 25. 25. 25. 25. 25. 25. 25. 25. 25.]
 [50. 50. 50. 50. 50. 50. 50. 50. 50. 50.]
 [75. 75. 75. 75. 75. 75. 75. 75. 75. 75.]]

Screenshots
If applicable, add screenshots to help explain your problem.

Environment Info:

• OS: Linux (debian sid)
• Satpy Version: 0.20.0
• PyResample Version: 1.14
• Readers and writers dependencies (when relevant):

$ env PYGAC_CONFIG_FILE=/etc/pygac.cfg python3 -c "from satpy.config import check_satpy; check_satpy()"
Readers
=======
abi_l1b:  ok
abi_l1b_scmi:  ok
abi_l2_nc:  ok
acspo:  ok
agri_l1:  ok
ahi_hrit:  ok
ahi_hsd:  ok
ami_l1b:  ok
amsr2_l1b:  ok
avhrr_l1b_aapp:  ok
avhrr_l1b_eps:  ok
avhrr_l1b_gaclac:  ok
avhrr_l1b_hrpt:  ok
caliop_l2_cloud:  ok
clavrx:  ok
electrol_hrit:  ok
fci_l1c_fdhsi:  ok
generic_image:  ok
geocat:  ok
ghrsst_l3c_sst:  ok
glm_l2:  ok
goes-imager_hrit:  ok
goes-imager_nc:  ok
grib:  ok
hsaf_grib:  ok
iasi_l2:  ok
iasi_l2_so2_bufr:  ok
jami_hrit:  ok
li_l2:  ok
maia:  ok
mersi2_l1b:  ok
mimicTPW2_comp:  ok
modis_l1b:  ok
modis_l2:  ok
msi_safe:  ok
mtsat2-imager_hrit:  ok
nucaps:  ok
nwcsaf-geo:  ok
nwcsaf-msg2013-hdf5:  ok
nwcsaf-pps_nc:  ok
olci_l1b:  ok
olci_l2:  ok
omps_edr:  ok
safe_sar_l2_ocn:  ok
sar-c_safe:  ok
scatsat1_l2b:  ok
seviri_l1b_hrit:  ok
seviri_l1b_icare:  ok
seviri_l1b_native:  ok
seviri_l1b_nc:  ok
seviri_l2_bufr:  ok
slstr_l1b:  ok
slstr_l2:  ok
tropomi_l2:  ok
vaisala_gld360:  ok
viirs_compact:  ok
viirs_edr_active_fires:  ok
viirs_edr_flood:  ok
viirs_l1b:  ok
viirs_sdr:  ok
virr_l1b:  ok

Writers
=======
/usr/lib/python3/dist-packages/pyninjotiff/tifffile.py:155: UserWarning: failed to import the optional _tifffile C extension module.
Loading of some compressed images will be slow.
Tifffile.c can be obtained at http://www.lfd.uci.edu/~gohlke/
  "failed to import the optional _tifffile C extension module.\n"
cf:  ok
geotiff:  ok
mitiff:  ok
ninjotiff:  ok
scmi:  ok
simple_image:  ok

Extras
======
cartopy:  ok
geoviews:  No module named 'geoviews'

[djhoese]

Very interesting. The new Satpy should definitely need the new pyninjotiff from what I've seen. I'm curious how this test is passing on the master branch as it doesn't look like area is defined in the DataArray object. Additionally, pyninjotiff should be doing img.data.attrs['area'] not img.data.area (both will work but the first way is preferred).

So the first question we need to answer is why this is passing in our automated testing and not for you. Let's see what @mraspaud has to say.

[djhoese]

Ok I did a little investigating...this test should never even touch the pyninjotiff package. it uses unittest.mock to mock the pyninjotiff module. So...it would seem something in the mock is not working for you. This works on my Ubuntu machine with current master of Satpy and I don't think I even have pyninjotiff installed.

[avalentino]

Thanks @djhoese for looking at this issue.
I can confirm that the test warks if I run it standalone:

$ python3 -m unittest -v satpy.tests.writer_tests.test_ninjotiff.TestNinjoTIFFWriter.test_image
test_image (satpy.tests.writer_tests.test_ninjotiff.TestNinjoTIFFWriter)
Test saving an image. ... 
nt <Mock name='mock.ninjotiff' id='139863332190224'>
nt.ImageWriter.save_image <Mock name='mock.ninjotiff.ImageWriter.save_image' id='139863516762448'>
NinjoTIFFWriter.save_dataset <MagicMock name='save_dataset' id='139863332188560'>
ntw <satpy.writers.ninjotiff.NinjoTIFFWriter object at 0x7f34783cbc10>
dataset <xarray.DataArray (dim_0: 3)>
array([1, 2, 3])
Dimensions without coordinates: dim_0
Attributes:
    units:    K
img <satpy.tests.writer_tests.test_ninjotiff.FakeImage object at 0x7f34783cee10>
ok

----------------------------------------------------------------------
Ran 1 test in 0.507s

OK

I have also added some print statements for debugging.

If I tun the entire test suite altogether I still have an error:

[CUT]

======================================================================
ERROR: test_image (satpy.tests.writer_tests.test_ninjotiff.TestNinjoTIFFWriter)
Test saving an image.
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/usr/lib/python3.7/unittest/mock.py", line 1676, in _inner
    return f(*args, **kw)
  File "/usr/lib/python3.7/unittest/mock.py", line 1255, in patched
    return func(*args, **keywargs)
  File "/home/antonio/debian/git/satpy/satpy/tests/writer_tests/test_ninjotiff.py", line 82, in test_image
    ret = ntw.save_image(img, filename='bla.tif', compute=False)
  File "/home/antonio/debian/git/satpy/satpy/writers/ninjotiff.py", line 155, in save_image
    return nt.save(img, filename, data_is_scaled_01=True, compute=compute, **kwargs)
  File "/usr/lib/python3/dist-packages/pyninjotiff/ninjotiff.py", line 541, in save
    area_def = img.data.area
  File "/usr/lib/python3/dist-packages/xarray/core/common.py", line 233, in __getattr__
    "{!r} object has no attribute {!r}".format(type(self).__name__, name)
AttributeError: 'DataArray' object has no attribute 'area'

----------------------------------------------------------------------
Ran 684 tests in 264.689s

FAILED (errors=1, skipped=6)
Test failed: <unittest.runner.TextTestResult run=684 errors=1 failures=0>

nt <Mock name='mock.ninjotiff' id='140199885636688'>
nt.ImageWriter.save_image <Mock name='mock.ninjotiff.ImageWriter.save_image' id='140200680830800'>
NinjoTIFFWriter.save_dataset <MagicMock name='save_dataset' id='140200684241552'>
ntw <satpy.writers.ninjotiff.NinjoTIFFWriter object at 0x7f82d45e3d50>
dataset <xarray.DataArray (dim_0: 3)>
array([1, 2, 3])
Dimensions without coordinates: dim_0
Attributes:
    units:    K
img <satpy.tests.writer_tests.test_ninjotiff.FakeImage object at 0x7f8303c73bd0>

error: Test failed: <unittest.runner.TextTestResult run=684 errors=1 failures=0>

Unfortunately I was not able to spot the problem.

[djhoese]

I wonder if the mocking of the modules fails if it's already in Python's cache of imported modules (from other tests). Something to look at.

[avalentino]

I wonder if the mocking of the modules fails if it's already in Python's cache of imported modules (from other tests). Something to look at.

Yes it is possible. In any case it seems to be something not too easy to track down.
In case you are interested in, there are also some other tests that have the same strange behaviour related to mocking. In [1] you can find the patch that I use in debian to disable those tests.

[1] https://salsa.debian.org/debian-gis-team/satpy/-/blob/d84d1a2073cfa6787a126f39ceb13876afdff474/debian/patches/0003-Skip-broken-tests.patch

[djhoese]

Just curious, do the tests fail if you uninstall pyninjotiff?

[djhoese]

Good news: I was able to get this to fail locally (Ubuntu laptop with conda environment and pyninjotiff installed) when I run all the tests. If I run just the ninjotiff writer tests it didn't seem to fail.

FYI @avalentino, we will be switching to pytest as our main way of starting tests (after #1095 is merged). This doesn't mean much for the existing tests but pytest satpy/tests will be the new way we start tests. Also note that python setup.py test is being deprecated in the setuptools world so will also be removing that.

Edit: Able to get it to fail with pyninjotiff and running pytest satpy/tests/test_writers.py satpy/tests/writer_tests/test_ninjotiff.py. This is probably caused by test_writers.py including the "available_writers" test which imports all possible writers (including ninjotiff).

[djhoese]

Figured it out: #1098

[avalentino]

@djhoese thanks for the quick fix.
Tests now pass both with setup.py test and pytest (when I apply the patch provided in #1098).
Tests fail, instead, if I remove pyninjotiff

[djhoese]

Thanks. I see the pull request tests failed too (pyninjotiff isn't installed for those). I just updated my pull request. See if this works better for you.

[avalentino]

@djhoese yes now it works both with and without pyninjotiff installed.
Thanks a lot