Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support to Pyspectral NIRReflectance masking limit #1390

Merged
merged 5 commits into from Oct 19, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
25 changes: 16 additions & 9 deletions satpy/composites/__init__.py
Expand Up @@ -45,6 +45,7 @@
from pyspectral.near_infrared_reflectance import Calculator
except ImportError:
Calculator = None

try:
from pyorbital.astronomy import sun_zenith_angle
except ImportError:
Expand Down Expand Up @@ -619,17 +620,25 @@ def __call__(self, projectables, optional_datasets=None, **info):
class NIRReflectance(CompositeBase):
"""Get the reflective part of NIR bands."""

def __init__(self, sunz_threshold=None, **kwargs):
TERMINATOR_LIMIT = 85.0
MASKING_LIMIT = 88.0

def __init__(self, sunz_threshold=TERMINATOR_LIMIT,
masking_limit=MASKING_LIMIT, **kwargs):
"""Collect custom configuration values.

Args:
sunz_threshold: The threshold sun zenith angle used when deriving
the near infrared reflectance. Above this angle the derivation
will assume this sun-zenith everywhere. Default None, in which
case the default threshold defined in Pyspectral will be used.
will assume this sun-zenith everywhere. Unless overridden, the
default threshold of 85.0 degrees will be used.
masking_limit: Mask the data (set to NaN) above this Sun zenith angle.
By default the limit is at 88.0 degrees. If set to `None`, no masking
is done.

"""
self.sun_zenith_threshold = sunz_threshold
self.masking_limit = masking_limit
super(NIRReflectance, self).__init__(**kwargs)

def __call__(self, projectables, optional_datasets=None, **info):
Expand Down Expand Up @@ -686,6 +695,7 @@ def _create_modified_dataarray(self, reflectance, base_dataarray):
proj = xr.DataArray(reflectance, dims=base_dataarray.dims,
coords=base_dataarray.coords, attrs=base_dataarray.attrs.copy())
proj.attrs['sun_zenith_threshold'] = self.sun_zenith_threshold
proj.attrs['sun_zenith_masking_limit'] = self.masking_limit
self.apply_modifier_info(base_dataarray, proj)
return proj

Expand All @@ -700,12 +710,9 @@ def _init_reflectance_calculator(self, metadata):
LOG.info("Couldn't load pyspectral")
raise ImportError("No module named pyspectral.near_infrared_reflectance")

if self.sun_zenith_threshold is not None:
reflectance_3x_calculator = Calculator(metadata['platform_name'], metadata['sensor'], metadata['name'],
sunz_threshold=self.sun_zenith_threshold)
else:
reflectance_3x_calculator = Calculator(metadata['platform_name'], metadata['sensor'], metadata['name'])
self.sun_zenith_threshold = reflectance_3x_calculator.sunz_threshold
reflectance_3x_calculator = Calculator(metadata['platform_name'], metadata['sensor'], metadata['name'],
sunz_threshold=self.sun_zenith_threshold,
masking_limit=self.masking_limit)
Comment on lines -703 to +715
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure this is right. When self.sun_zenith_threshold is None, we want to replace it with the default value from the calculator.
This was what #1319 was about.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh. That sounds wrong. I'd expect None to mean "do not apply SZA correction in the Calculator". I'd just fix the documentation to say If not explicitly specified, the default value defined in Pyspectral will be used.

But anyway, the behaviour on the Satpy side doesn't change: the default value is used if the user doesn't override it. If the user decides to override the default with None, they'll probably know what they are doing (which is, crash the code 😁).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok, just change the __init__ docstring then :)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Docstring updated. Good thing, as I didn't remember to add the new kwarg there before.

return reflectance_3x_calculator


Expand Down
42 changes: 39 additions & 3 deletions satpy/tests/test_composites.py
Expand Up @@ -648,7 +648,8 @@ def test_provide_sunz_and_threshold(self, calculator, apply_modifier_info, sza):
res = comp([self.nir, self.ir_], optional_datasets=[self.sunz], **info)

self.assertEqual(res.attrs['sun_zenith_threshold'], 84.0)
calculator.assert_called_with('Meteosat-11', 'seviri', 'IR_039', sunz_threshold=84.0)
calculator.assert_called_with('Meteosat-11', 'seviri', 'IR_039',
sunz_threshold=84.0, masking_limit=NIRReflectance.MASKING_LIMIT)

@mock.patch('satpy.composites.sun_zenith_angle')
@mock.patch('satpy.composites.NIRReflectance.apply_modifier_info')
Expand All @@ -657,14 +658,47 @@ def test_sunz_threshold_default_value_is_not_none(self, calculator, apply_modifi
"""Check that sun_zenith_threshold is not None."""
from satpy.composites import NIRReflectance

comp = NIRReflectance(name='test', sunz_threshold=None)
comp = NIRReflectance(name='test')
info = {'modifiers': None}
calculator.return_value = mock.MagicMock(
reflectance_from_tbs=self.refl_from_tbs)
comp([self.nir, self.ir_], optional_datasets=[self.sunz], **info)

assert comp.sun_zenith_threshold is not None

@mock.patch('satpy.composites.sun_zenith_angle')
@mock.patch('satpy.composites.NIRReflectance.apply_modifier_info')
@mock.patch('satpy.composites.Calculator')
def test_provide_masking_limit(self, calculator, apply_modifier_info, sza):
"""Test NIR reflectance compositor provided sunz and a sunz threshold."""
calculator.return_value = mock.MagicMock(
reflectance_from_tbs=self.refl_from_tbs)
from satpy.composites import NIRReflectance
sza.return_value = self.da_sunz

comp = NIRReflectance(name='test', masking_limit=None)
info = {'modifiers': None}
res = comp([self.nir, self.ir_], optional_datasets=[self.sunz], **info)

self.assertIsNone(res.attrs['sun_zenith_masking_limit'])
calculator.assert_called_with('Meteosat-11', 'seviri', 'IR_039',
sunz_threshold=NIRReflectance.TERMINATOR_LIMIT, masking_limit=None)

@mock.patch('satpy.composites.sun_zenith_angle')
@mock.patch('satpy.composites.NIRReflectance.apply_modifier_info')
@mock.patch('satpy.composites.Calculator')
def test_masking_limit_default_value_is_not_none(self, calculator, apply_modifier_info, sza):
"""Check that sun_zenith_threshold is not None."""
from satpy.composites import NIRReflectance

comp = NIRReflectance(name='test')
info = {'modifiers': None}
calculator.return_value = mock.MagicMock(
reflectance_from_tbs=self.refl_from_tbs)
comp([self.nir, self.ir_], optional_datasets=[self.sunz], **info)

assert comp.masking_limit is not None


class TestNIREmissivePartFromReflectance(unittest.TestCase):
"""Test the NIR Emissive part from reflectance compositor."""
Expand All @@ -677,6 +711,7 @@ def test_compositor(self, calculator, apply_modifier_info, sza):
import numpy as np
import xarray as xr
import dask.array as da
from satpy.composites import NIRReflectance

refl_arr = np.random.random((2, 2))
refl = da.from_array(refl_arr)
Expand Down Expand Up @@ -728,7 +763,8 @@ def test_compositor(self, calculator, apply_modifier_info, sza):
self.assertEqual(res.attrs['platform_name'], platform)
self.assertEqual(res.attrs['sensor'], sensor)
self.assertEqual(res.attrs['name'], chan_name)
calculator.assert_called_with('NOAA-20', 'viirs', 'M12', sunz_threshold=86.0)
calculator.assert_called_with('NOAA-20', 'viirs', 'M12', sunz_threshold=86.0,
masking_limit=NIRReflectance.MASKING_LIMIT)


class TestColormapCompositor(unittest.TestCase):
Expand Down
2 changes: 1 addition & 1 deletion setup.py
Expand Up @@ -51,7 +51,7 @@
'amsr2_l1b': ['h5py >= 2.7.0'],
'hrpt': ['pyorbital >= 1.3.1', 'pygac', 'python-geotiepoints >= 1.1.7'],
'proj': ['pyresample'],
'pyspectral': ['pyspectral >= 0.8.7'],
'pyspectral': ['pyspectral >= 0.10.1'],
'pyorbital': ['pyorbital >= 1.3.1'],
'hrit_msg': ['pytroll-schedule'],
'nc_nwcsaf_msg': ['netCDF4 >= 1.1.8'],
Expand Down