DOI-USGS / knoten

Python Geospatial Sensor Exploitation Library
Other
3 stars 21 forks source link

LatLon Footprint Generation Requires Superfluous Argument #135

Open jlaura opened 1 month ago

jlaura commented 1 month ago

The func requires that an image boundary be passed in: https://github.com/DOI-USGS/knoten/blob/main/knoten/csm.py#L296 along side the sensor model. Instead of pushing the user to make multiple calls, simple get the images size from the sensor model and then allow user to pass linc, sinc args (please not those names) like ISIS accepts.

The current requirement:

isize = sensor.getImageSize()
print(isize.samp, isize.line)
generate_latlon_footprint(sensor, [isize.samp, isize.line], dem=dem, radii=3396190)

A better func:

generate_latlon_footprint(sensor,dem=dem, radii=3396190, nlines, nsmaples)
jlaura commented 1 month ago

Further, the call requires the radii. This is redundant as the sensor model already encodes the semimajor and semiminor axes.

All args:

generate_latlon_footprint(sensor, dem=dem, nlines, nsmaples)

Defaults:

generate_latlon_footprint(sensor, dem)
acpaquette commented 1 month ago

Hey Jay, I think there is room for some QOL stuff in knoten. Does the following function accomplish a similarly desired goal as the nlines and nsamples? https://github.com/DOI-USGS/knoten/blob/459231a42ab26a57ce9f70528d990d739e0650ae/knoten/csm.py#L273

It does look like the generate_latlon_footprint is bugged where radii remains None after we try to account for it here: https://github.com/DOI-USGS/knoten/blob/459231a42ab26a57ce9f70528d990d739e0650ae/knoten/csm.py#L408

It looks like the expected arg/function flow is as follows:

camera = create_csm(image, verbose=False)
boundary = generate_boundry(camera, 20)
# after default radii is fixed
footprint = generate_latlon_footprint(camera, boundary, dem=dem)

From the above, providing a nlines and nsamples arg in generate_boundry would be a positive. We could also generate the boundary by default for a user is none is provided

jlaura commented 1 month ago

@acpaquette Yes, yes, and yes. All of the pieces are there, we just need to wrap it up into a nicer to use function. I'll see if I can get a PR in pre-vacay. If not, an enhancement to consider at some point.