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

fix landsat 4/5 fft filename bug #164

Merged
merged 3 commits into from
Oct 10, 2022
Merged

fix landsat 4/5 fft filename bug #164

merged 3 commits into from
Oct 10, 2022

Conversation

forrestfwilliams
Copy link
Contributor

This PR makes a small change to the file path that FFT-filtered images are written to. Instead of writing them to /working/directory/{BASE_NAME}_fft.tif, we now write them to /working/directory/fft/{BASE_NAME}.tif. This ensures that routines that rely on string indexes of standard Landsat image names work correctly.

@forrestfwilliams forrestfwilliams requested a review from a team as a code owner October 10, 2022 13:48
jtherrmann
jtherrmann previously approved these changes Oct 10, 2022
Copy link
Contributor

@jtherrmann jtherrmann left a comment

Choose a reason for hiding this comment

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

Looks good, just wanted to confirm there are no tests to update?

@forrestfwilliams
Copy link
Contributor Author

forrestfwilliams commented Oct 10, 2022

No not currently, but the fact that we have no tests for this functionality may be a bigger issue.

@forrestfwilliams
Copy link
Contributor Author

@jhkennedy and @jtherrmann, currently the create_fft_filepath function creates the directory in which the data is saved. I'm concerned this may be an inappropriate side effect for this function. Thoughts?

@jtherrmann
Copy link
Contributor

@jhkennedy and @jtherrmann, currently the create_fft_filepath function creates the directory in which the data is saved. I'm concerned this may be an inappropriate side effect for this function. Thoughts?

@forrestfwilliams Yeah it may be better to create the directory right after this line, since it should be the same directory for both calls of the create_fft_filepath function, right? So no need to attempt to create it twice.

@jtherrmann jtherrmann dismissed their stale review October 10, 2022 20:32

needs more work (see comments)

hyp3_autorift/process.py Outdated Show resolved Hide resolved
Co-authored-by: Joseph H Kennedy <me@jhkennedy.org>
@jhkennedy jhkennedy merged commit 5eecad5 into develop Oct 10, 2022
@jhkennedy jhkennedy deleted the landsat45_name_bug branch October 10, 2022 21:28
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.

None yet

3 participants