-
Notifications
You must be signed in to change notification settings - Fork 32
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
Improve ASTER sensor angle calculation. #365
Conversation
tmieslinger
commented
Apr 21, 2020
- make sensor geometry calculation more efficient and more generic. Apprication is extended to ascending satellite mode (before only descending was covered).
- add sensor geometry for backward looking telescope of channel 3.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The PR looks fine in general 👍 I left some minor picky comments that you might want to incorporate ;)
Disclaimer: I haven't looked into the logic of the calculations.
typhon/cloudmask/aster.py
Outdated
@@ -617,79 +619,100 @@ def sensor_angles(self, sensor="vnir"): | |||
Algorithm Theoretical Basis Document for ASTER Level-1 Data | |||
Processing (Ver. 3.0).1996. | |||
""" | |||
if channel is not "3B": |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point.Thanks.
typhon/cloudmask/aster.py
Outdated
zenith = np.rad2deg(np.arctan(np.sqrt((h * np.tan(np.deg2rad(P[sensor])) | ||
+ 15 * n)**2 | ||
+ (h * np.tan(np.deg2rad(27.6)) | ||
/ np.cos(np.deg2rad(P[sensor])))**2 | ||
) / h | ||
) | ||
) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Did you or your IDE/code formatter chose that style? ;)
You could add a new line before h * np.tan...
this gives you way more space to operate in the next line
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
personal choice :) but I'm open to the back
suggestions
typhon/cloudmask/aster.py
Outdated
FOV = {"VNIR": 6.09, | ||
"VNIRB": 5.19, | ||
"SWIR": 4.9, | ||
"TIR":4.9 | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Picky comment from my side ;) There is two options to format code like this:
- If you know that the dictionary is complete:
FOV = {"VNIR": 6.09, "VNIRB": 5.19, "SWIR": 4.9, "TIR": 4.9}
- If it may be extended in the future:
FOV = {
"VNIR": 6.09,
"VNIRB": 5.19,
"SWIR": 4.9,
"TIR": 4.9,
}
typhon/cloudmask/aster.py
Outdated
"""Calculate sensor zenith and azimuth angles. | ||
Angles are derived for each pixel depending on the channel and the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Multi-line docstrings consist of a summary line just like a one-line docstring, followed by a blank line, followed by a more elaborate description. PEP257
* make sensor geometry calculation more efficient and more generic. Apprication is extended to ascending satellite mode (before only descending was covered). * add sensor geometry for backward looking telescope of channel 3.
659d565
to
fdf96ec
Compare