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 compositor for adding an image as a background #804

Merged
merged 28 commits into from Jun 12, 2019

Conversation

pnuu
Copy link
Member

@pnuu pnuu commented Jun 4, 2019

This PR adds two compositors that makes it possible to use pre-made images as a part of other composites:

  • StaticImageCompositor - load image and use it as a part of other composites
  • BackgroundCompositor - use a composite as a background for other composites

Add compositor for loading pre-made images as part of a composite.

@pnuu pnuu added enhancement code enhancements, features, improvements component:compositors labels Jun 4, 2019
satpy/composites/__init__.py Outdated Show resolved Hide resolved
satpy/composites/__init__.py Outdated Show resolved Hide resolved
@pnuu
Copy link
Member Author

pnuu commented Jun 4, 2019

I need some help to figure out how to make Satpy to recognize the composites based on StaticImageCompositor. Here's an example composite:

  background:
    compositor: !!python/name:satpy.composites.StaticImageCompositor
    standard_name: background
    fname: /tmp/overview.tif
    # Optional for non-GeoTIFF image file
    # area: euro4

And here's a test script (use whatever data and area available):

import glob
from satpy import Scene

fnames = glob.glob('/home/lahtinep/data/satellite/new/H*201409040500*')
glbl = Scene(reader='seviri_l1b_hrit', filenames=fnames)
glbl.load(['background'])
lcl = glbl.resample('euro4')
lcl.save_dataset('background', filename='/tmp/test.tif')

I get this stack trace:

Traceback (most recent call last):
  File "/home/lahtinep/Software/pytroll/packages/satpy/satpy/readers/__init__.py", line 335, in __getitem__
    return super(DatasetDict, self).__getitem__(item)
KeyError: 'background'

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/tmp/test.py", line 14, in <module>
    lcl.save_dataset('background', filename='/tmp/test.png')
  File "/home/lahtinep/Software/pytroll/packages/satpy/satpy/scene.py", line 1257, in save_dataset
    return writer.save_dataset(self[dataset_id],
  File "/home/lahtinep/Software/pytroll/packages/satpy/satpy/scene.py", line 680, in __getitem__
    return self.datasets[key]
  File "/home/lahtinep/Software/pytroll/packages/satpy/satpy/readers/__init__.py", line 337, in __getitem__
    key = self.get_key(item)
  File "/home/lahtinep/Software/pytroll/packages/satpy/satpy/readers/__init__.py", line 326, in get_key
    best=best, **dfilter)
  File "/home/lahtinep/Software/pytroll/packages/satpy/satpy/readers/__init__.py", line 277, in get_key
    raise KeyError("No dataset matching '{}' found".format(str(key)))
KeyError: "No dataset matching 'DatasetID(name='background', wavelength=None, resolution=None, polarization=None, calibration=None, level=None, modifiers=None)' found"

If I instead use the compositor directly and place the composite to the scene resampling and saving works:

import glob
from satpy import Scene

from satpy.composites import StaticImageCompositor
comp = StaticImageCompositor('static', fname='/tmp/overview.tif')
foo = comp()

fnames = glob.glob('/home/lahtinep/data/satellite/new/H*201409040500*')
glbl = Scene(reader='seviri_l1b_hrit', filenames=fnames)

glbl['foo'] = foo
lcl = glbl.resample('euro4')
lcl.save_dataset('foo', filename='/tmp/test.tif')

@djhoese
Copy link
Member

djhoese commented Jun 4, 2019

So I've tracked down the issue for why this doesn't behave the way you expect. The main issue is that the dependency tree, once constructed, is used in two ways:

  1. "Give me all the leaves of the tree": Nodes/datasets that do not depend on anything else and try to load them from the existing readers.
  2. "Give me all the trunk nodes of the tree": Nodes/datasets that depend on "leaf" nodes where we get prerequisites and then call the compositor object.

We have three easy options and one "No please don't make me do it" option for a fix.

  1. Add a special "None"-like node that says "This doesn't depend on anything else, but is still considered a composite". This requires special handling in the dependency tree to not return the None node when providing "leaves" and to not try to load a None node. The None node can be assigned any time there are no prerequisites.
  2. Modify the generic_image reader to be easier to use for cases like this so that you can name your image (via keyword argument passed to the reader)? Still may require manual loading from one Scene to another Scene that is doing the .load.
  3. Change the Background compositor to load the fname if provided...crap this won't work when resampling is needed.
  4. "No please": refactor the dependency tree to know the difference between something that comes from a reader and something that doesn't in a way independent of where it ends up on the tree.

I vote for 1 which I can try to implement if you and @mraspaud agree.

Also, could you rename fname to filename. Not a big fan of abbreviations in interfaces (except for maybe cmap but even then I don't know).

Handles the special case of compositors with no required dependencies
@djhoese
Copy link
Member

djhoese commented Jun 4, 2019

I've pushed fixes for option 1 above to your branch. Tests that need to be added and I think using your compositor is the best way to test them:

  1. Test that loading an image updates the name attribute of the DataArray returned. Previously, before my commit, it still had the image name provided by the generic_image reader.
  2. Test that the static image composite shows in the available_composite_names and in all_composite_names, but not in all_dataset_names.
  3. Test that it can load from the Scene.

@pnuu
Copy link
Member Author

pnuu commented Jun 6, 2019

Couldn't yet figure out what and how to mock to get real results from the scn.available_*() methods. On ipython prompt it's easy to test with real data, but gets rather complicated without any files :-) All the tests @djhoese listed work in real world, but mocking things is hard. Will have another go tomorrow.

@pnuu pnuu removed the help wanted label Jun 7, 2019
@pnuu pnuu marked this pull request as ready for review June 7, 2019 06:54
@coveralls
Copy link

coveralls commented Jun 7, 2019

Coverage Status

Coverage increased (+0.9%) to 83.201% when pulling a39799c on pnuu:feature-static-image-compositor into 6f1f451 on pytroll:master.

@pnuu
Copy link
Member Author

pnuu commented Jun 7, 2019

Here's an example (global version of the one given as example in composite documentation) image using DayNightCompositor, StaticImageCompositor and BackgroundCompositor.

background_compositor_example

@codecov
Copy link

codecov bot commented Jun 7, 2019

Codecov Report

Merging #804 into master will increase coverage by 0.93%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff            @@
##           master    #804      +/-   ##
=========================================
+ Coverage   82.26%   83.2%   +0.93%     
=========================================
  Files         159     163       +4     
  Lines       22998   23829     +831     
=========================================
+ Hits        18920   19826     +906     
+ Misses       4078    4003      -75
Impacted Files Coverage Δ
satpy/composites/__init__.py 73.06% <100%> (+3.82%) ⬆️
satpy/scene.py 90.19% <100%> (+0.1%) ⬆️
satpy/tests/compositor_tests/__init__.py 99.52% <100%> (+0.1%) ⬆️
satpy/node.py 94.5% <100%> (+3.08%) ⬆️
satpy/tests/utils.py 97.5% <100%> (+0.2%) ⬆️
satpy/tests/test_scene.py 99.46% <100%> (+0.01%) ⬆️
satpy/writers/utils.py 100% <0%> (ø)
satpy/tests/reader_tests/test_vaisala_gld360.py 96% <0%> (ø)
satpy/tests/writer_tests/test_utils.py 93.33% <0%> (ø)
satpy/readers/vaisala_gld360.py 90% <0%> (ø)
... and 18 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 6f1f451...a39799c. Read the comment docs.

Copy link
Member

@djhoese djhoese left a comment

Choose a reason for hiding this comment

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

LGTM. So what does this mean for the start_time issue? If the image doesn't have a time in the filename that can be picked up by the reader then users are screwed? Could someone provide a datetime as a keyword argument to the compositor?

@pnuu
Copy link
Member Author

pnuu commented Jun 7, 2019

If the image file has no start_time derived from the filename, dt.datetime.utcnow() is used. This works great for NRT operational production, but the kwarg you suggest would be a great adfition. Actually, there might be some cases where it could have use for all the compositors (mainly DayNightCompositor, and testing modifiers for time slots where one doesn't have ctual data), so maybe it could be added to GenericCompositor in another PR?

@djhoese
Copy link
Member

djhoese commented Jun 7, 2019

@pnuu I'm not sure I understand how it would be used by other compositors, but waiting for a later PR seems fine. Actually, it may even be possible to just add start_time to the compositor parameters in a nice hacky way like: https://stackoverflow.com/a/4206943/433202

Copy link
Member

@mraspaud mraspaud left a comment

Choose a reason for hiding this comment

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

LGTM

@mraspaud mraspaud merged commit ea9b4e6 into pytroll:master Jun 12, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component:compositors enhancement code enhancements, features, improvements
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add a compositor that can use static images Creating day/night composites
5 participants