Skip to content

Comments

i.sentinel1.pyrosargeocode: Use current region to define AOI and pixel spacing#47

Open
griembauer wants to merge 2 commits intoNVE:mainfrom
griembauer:isentinelpyrosar_pspacing
Open

i.sentinel1.pyrosargeocode: Use current region to define AOI and pixel spacing#47
griembauer wants to merge 2 commits intoNVE:mainfrom
griembauer:isentinelpyrosar_pspacing

Conversation

@griembauer
Copy link

This PR adds a -a flag to i.sentinel1.pyrosargeocode, which will then use the current computational region to define the AOI and the pixel spacing.

Copy link
Collaborator

@ninsbl ninsbl left a comment

Choose a reason for hiding this comment

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

Thanks for the nice addition! I am very positive to this change! Please have a look at my one comment. If there is a good reason for using temporary maps, I will merge as is...

Comment on lines +554 to +566
region_vect = f"cur_region_{os.getpid()}"
rm_vec.append(region_vect)
gs.run_command("v.in.region", output=region_vect, quiet=True)
cur_reg_json = f"{gs.tempfile(create=False)}.geojson"
rm_files.append(cur_reg_json)
gs.run_command("v.out.ogr", input=region_vect, output=cur_reg_json,
format="GeoJSON", quiet=True)
# reproject to 4326
reg_json_4326 = f"{gs.tempfile(create=False)}_4326.geojson"
rm_files.append(reg_json_4326)
ds = gdal.VectorTranslate(reg_json_4326, cur_reg_json, reproject=True, dstSRS="EPSG:4326")
ds = None
aoi = reg_json_4326
Copy link
Collaborator

Choose a reason for hiding this comment

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

Did you consider using g.region -b to get region bounds in lat/long directly and and then write it as WKT to GeoJSON? That would mean less temporary files and maps...

Copy link
Author

Choose a reason for hiding this comment

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

Thanks! I considered this too but the suggested way in this PR seemed less "invasive" as a WKT generated from region -b directly would need to be passed to get_target_geometry, which would mean passing more optional stuff through functions. The way it is implemented in this PR, the new region AOI geojson is created once in the beginning and all the remaining pipeline remains untouched.
So I suppose there is no strong reason to do it this way, but just the attempt to alter the existing code as little as possible.

Comment on lines +554 to +566
region_vect = f"cur_region_{os.getpid()}"
rm_vec.append(region_vect)
gs.run_command("v.in.region", output=region_vect, quiet=True)
cur_reg_json = f"{gs.tempfile(create=False)}.geojson"
rm_files.append(cur_reg_json)
gs.run_command("v.out.ogr", input=region_vect, output=cur_reg_json,
format="GeoJSON", quiet=True)
# reproject to 4326
reg_json_4326 = f"{gs.tempfile(create=False)}_4326.geojson"
rm_files.append(reg_json_4326)
ds = gdal.VectorTranslate(reg_json_4326, cur_reg_json, reproject=True, dstSRS="EPSG:4326")
ds = None
aoi = reg_json_4326
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
region_vect = f"cur_region_{os.getpid()}"
rm_vec.append(region_vect)
gs.run_command("v.in.region", output=region_vect, quiet=True)
cur_reg_json = f"{gs.tempfile(create=False)}.geojson"
rm_files.append(cur_reg_json)
gs.run_command("v.out.ogr", input=region_vect, output=cur_reg_json,
format="GeoJSON", quiet=True)
# reproject to 4326
reg_json_4326 = f"{gs.tempfile(create=False)}_4326.geojson"
rm_files.append(reg_json_4326)
ds = gdal.VectorTranslate(reg_json_4326, cur_reg_json, reproject=True, dstSRS="EPSG:4326")
ds = None
aoi = reg_json_4326
reg_bounds = gs.parse_command("g.region", flags="lg")
reg_json_4326 = Path(f"{gs.tempfile(create=False)}_4326.geojson")
reg_json_4326.write_text(
json.dumps(
{
"type": "FeatureCollection",
"name": "region_aoi",
"crs": {
"type": "name",
"properties": {"name": "urn:ogc:def:crs:OGC:1.3:CRS84"},
},
"features": [
{
"type": "Feature",
"properties": {},
"geometry": {
"type": "Polygon",
"coordinates": [
[
[
[
reg_bounds["nw_long"],
reg_bounds["nw_lat"],
],
[
reg_bounds["sw_long"],
reg_bounds["sw_lat"],
],
[
reg_bounds["se_long"],
reg_bounds["se_lat"],
],
[
reg_bounds["ne_long"],
reg_bounds["ne_lat"],
],
[
reg_bounds["nw_long"],
reg_bounds["nw_lat"],
],
]
]
],
},
}
],
},
indent=2,
),
encoding="UTF8",
)
aoi = str(reg_json_4326)

Copy link
Collaborator

Choose a reason for hiding this comment

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

Here is my line of thinking. Do you think this may work?
Then, atexit procedure might not be needed either...

# %end

import atexit
import os
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
import os
import json
import os

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants