MTG LI L2: Hard coded the satellite position with the nominal vallues #3137
MTG LI L2: Hard coded the satellite position with the nominal vallues #3137jequierz wants to merge 5 commits intopytroll:mainfrom
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #3137 +/- ##
=======================================
Coverage 96.22% 96.22%
=======================================
Files 398 398
Lines 57383 57388 +5
=======================================
+ Hits 55218 55223 +5
Misses 2165 2165
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Pull Request Test Coverage Report for Build 15140857231Details
💛 - Coveralls |
gerritholl
left a comment
gerritholl
left a comment
There was a problem hiding this comment.
Still needs a unit test, and I suggested some other changes.
satpy/readers/li_base_nc.py
Outdated
| logger.info("The satellite position is the nominal position and thus does not" \ | ||
| "correspond to the actual position of the satellite. The exact satellite position can be found in " \ | ||
| "FCI level 1 data. ") |
There was a problem hiding this comment.
I find the formulation here unclear. We should remember that logging messages may appear out of context for most users (they don't call add_orbital_parameters directly, but it gets called as part of larger processing). Maybe something like:
Assuming nominal satellite position for LI L2. The exact position is available from FCI L1.
And I would report this in debug mode, not info mode, as most users will not care about the difference. One use case is parallax correction — for this use case, the difference is too small to be relevant.
There was a problem hiding this comment.
Thank you for the suggestion. I will change this :)
| data_array.attrs["orbital_parameters"] = {"satellite_nominal_longitude": 0.0, | ||
| "satellite_nominal_latitude": 0.0, | ||
| "satellite_nominal_altitude": 35786400.0 | ||
| } |
There was a problem hiding this comment.
If at least the satellite number is known from the metadata (is it?) this hardcoded position could be sorted by satellite. MTG-I2 / Meteosat-14 will have a different position.
There was a problem hiding this comment.
Good point, I didn't think about the fact that other satellite's products would require the use of the LI reader... But it is true that in any case, it would be better to have a more general solution because apparently the same problem holds for any L2 product of the MTG.
However, I am not sure to understand, how does the satellite number can be used to for this application? By writing a dict of satellites containing a dict of nominal position for each satellite?
There was a problem hiding this comment.
Yes, I would have a dict of satellites. Those should rarely change, although if they do, it will cause more of a headache as we would need to cover old and new data.
There was a problem hiding this comment.
yeah, when we start moving the MTGs between the nominal, rapid scan , IODC position etc, we'll have a new headache. But maybe by then we can include the information in the file format... So for now a simple dict mapping the platform_name to the satellite_nominal_longitude should be good for a while. Note that the platform_name was added only recently in #2993
satpy/readers/li_base_nc.py
Outdated
|
|
||
| def add_orbital_parameters(self, data_array): | ||
| """Hard code the nominal position of the satellite in the orbital parameters attribute.""" | ||
| logger.info("The satellite position is the nominal position and thus does not" \ |
There was a problem hiding this comment.
| logger.info("The satellite position is the nominal position and thus does not" \ | |
| logger.info("The satellite position is the nominal position and thus does not " |
without a trailing space (or a leading one on the next line), it's going to output "does notcorrespond to".
satpy/readers/li_base_nc.py
Outdated
| def add_orbital_parameters(self, data_array): | ||
| """Hard code the nominal position of the satellite in the orbital parameters attribute.""" | ||
| logger.info("The satellite position is the nominal position and thus does not" \ | ||
| "correspond to the actual position of the satellite. The exact satellite position can be found in " \ |
There was a problem hiding this comment.
| "correspond to the actual position of the satellite. The exact satellite position can be found in " \ | |
| "correspond to the actual position of the satellite. The exact satellite position can be found in " |
satpy/readers/li_base_nc.py
Outdated
| # Update the attributes in the final array: | ||
| data_array = self.update_array_attributes(data_array, ds_info) | ||
|
|
||
| #Add the nominal sat position in attrs orbital parameters |
There was a problem hiding this comment.
| #Add the nominal sat position in attrs orbital parameters | |
| # Add the nominal sat position in attrs orbital parameters |
4 participants
In the MTG LI L2 reader
li_l2_nc, I have hard coded the nominal satellite position as given in the L1 FCI dataset. It allows to use functions such asget_satposfromsatpy.utilsthat is necessary to apply a parallax correction to the data.