Skip to content
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

socetnet2isis and isisnet2socet Applying Uneccessary 0.5 pixel shifts #135

Closed
dpmayerUSGS opened this issue Oct 9, 2019 · 5 comments · Fixed by #136
Closed

socetnet2isis and isisnet2socet Applying Uneccessary 0.5 pixel shifts #135

dpmayerUSGS opened this issue Oct 9, 2019 · 5 comments · Fixed by #136
Assignees

Comments

@dpmayerUSGS
Copy link
Contributor

dpmayerUSGS commented Oct 9, 2019

As of #127 , plio has adopted a (0,0) pixel origin. Currently, the socetnet2isis and isisnet2socet bin scripts perform both an origin shift and a pixel origin correction. Based on the changes made to to_isis() and from_isis() in #127, the pixel origin correction in the bin scripts is now in error by 0.5 pixels in both the sample and line directions.

To fix this, the bin scripts should be changed to apply a pixel origin correction with a magnitude of 0.5 pixels, rather than 1.0 pixels. This should be as simple as modifying the following lines in the bin scripts:

https://github.com/USGS-Astrogeology/plio/blob/185a854774820cd0fc98c5621b4534fbf07f1d57/bin/socetnet2isis#L66-L67

https://github.com/USGS-Astrogeology/plio/blob/185a854774820cd0fc98c5621b4534fbf07f1d57/bin/isisnet2socet#L126-L127

@jessemapel
Copy link
Contributor

@dpmayerUSGS Are you comfortable putting up a PR as you've already identified the fix?

@dpmayerUSGS
Copy link
Contributor Author

@jessemapel That depends. What else would the PR need to contain other than edits to those 4 lines in order to be accepted and merged?

@jessemapel
Copy link
Contributor

I'm fairly sure the scripts are not actually tested in the repo so that's it.

@dpmayerUSGS
Copy link
Contributor Author

I can confirm that the scripts are not tested. I'll self-assign this issue.

@dpmayerUSGS dpmayerUSGS self-assigned this Oct 15, 2019
@jessemapel
Copy link
Contributor

Either @tthatcher95 or @acpaquette Can review as they originally worked on the scripts.

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 a pull request may close this issue.

2 participants