Closed vsoch closed 3 years ago
@vsoch - Are you okay with having mini, unit-test specific recipe files added for testing clean? If so, would a new "resources" folder under tests be okay?
Definitely!
I have some unit tests written as well as some bug fixes. They're currently committed here, but I can port them to where ever you want if you are okay with the changes.
Just to point out a couple things:
Great! Let's do the PR against the branch here, then we can merge, make sure all the tests still work, and then merge again and release. Everything sounds good except for:
Removed the "cleaned-" prefix from the output header files. This simplifies the merging of the pixel output with header output.
The concern here is that the user would accidentally rewrite files - I think we should keep the cleaned- prefix. It's also a message that the files were absolutely generated - without the prefix it might not be clear. If you feel strongly that this isn't a good feature, then we could have some kind of input argument for an output prefix, with default to be cleaned-
and for your use case setting it to an empty string.
Got it.
I don't have a strong preference on the "cleaned-" prefix. Let's just leave it as is.
Same here. I will be continuing to test. If I see anything else I'll open another issue.
I'll give you a day or so to test and report any quick issues, otherwise I'll release after that!
Description
Opening this PR as start of work to refactor the detect and clean flow, changes include:
cc @wetzelj
Related issues: #149
Please include a summary of the change(s) and if relevant, any related issues above.
Checklist
Open questions
Questions that require more discussion or to be addressed in future development: