-
Notifications
You must be signed in to change notification settings - Fork 147
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
i.sentinel.download: various improvements #1044
Conversation
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 test but it looks good
# for details. | ||
# | ||
############################################################################# | ||
|
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.
What is the reason for changing header (comments to multi-line string)?
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.
pylint complained about missing module docstring. I am not sure if there is a prefered way for how the module header should be written. I happily change it back if that is prefered...
src/imagery/i.sentinel/i.sentinel.download/i.sentinel.download.py
Outdated
Show resolved
Hide resolved
|
||
import grass.script as gs | ||
from grass.pygrass.modules import Module |
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.
Personally I prefer PyGRASS API. But what is the reason to use it on single place only?
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.
I wanted to pipe to stdin of m.proj
and use the results in stdout
at the same time, see:
grass-addons/src/imagery/i.sentinel/i.sentinel.download/i.sentinel.download.py
Lines 282 to 297 in 4a8e44e
coord_proj = Module( | |
"m.proj", | |
input="-", | |
flags="od", | |
stdin_="\n".join(coords), | |
stdout_=PIPE, | |
stderr_=PIPE, | |
) | |
poly += (", ").join( | |
[ | |
" ".join(poly_coords.split("|")[0:2]) | |
for poly_coords in coord_proj.outputs["stdout"] | |
.value.strip() | |
.split("\n") | |
] | |
) + "))" |
I am not aware of how to do that in the script API, while I knoew it to work in pygrass. If you have a suggestion for how to do this, I am happy to adjust...
src/imagery/i.sentinel/i.sentinel.download/i.sentinel.download.py
Outdated
Show resolved
Hide resolved
@landam OK to merge? |
Since @HamedElgizery will be working on it later on during GSoC, it would be great indeed to have this merged |
I am currently using the version from my branch without issues. Taking the liberty to merge. |
This PR adds: