pytroll / satpy

Python package for earth-observing satellite data processing
http://satpy.readthedocs.org/en/latest/
GNU General Public License v3.0
1.04k stars 287 forks source link

Using a static image alters time information #2734

Closed pnuu closed 4 months ago

pnuu commented 5 months ago

Feature Request

Creating a composite that uses a static background image yields an image with a different time than the used satellite data. The current approach sets the start_time of the static image to utcnow(), and satpy.dataset.combine_metadata takes average of this and the other dataset used in the creation of the composite. Creating images in NRT might not change the time stamp a lot, but doing the same for data 2 days ago back already shifts the result by one full day.

Describe the solution you'd like Instead of always taking the average of the times, could we instead have multiple options:

Could we use the Satpy config system for controlling this? I would argue the default should be min(), but I'm open to comments.

Implementation it self is simple: add new min/max functions and call min/mean/max depending on the setting.

Describe any changes to existing user workflow No compatibility issues if mean is kept as default. Setting the config item would need one additional line of code in scripts when required.

The only place where combine_metadata(..., average_times=False) is used is in a test method.

Additional context I was thinking this could also be a bug, but in the end decided a feature request would be better instead.

djhoese commented 5 months ago

Kind of a duplicate of https://github.com/pytroll/satpy/issues/2630

I think the question is whether or not this is an option for all datetime types in the metadata or just start_time and end_time. There are other times in the metadata now (observation, nominal, etc), but if we're merging datasets and these differ do we really want to min/max/mean these at all? The alternative is to always min the start_time and max the end_time.

pnuu commented 5 months ago

Damn, missed #2630 .

I'd be happy with automatic min(start_times) and max(end_times). Currently all attributes having "time" in their names are averaged, so as a first guess we could keep those as-is.

I'll create a PR and link these two issues to it.

djhoese commented 5 months ago

Or maybe support of a None start time would be better? Less "fake" data.

pnuu commented 5 months ago

I added also handling for None as time. PR coming in a second.