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

Enforce exact_match=True when listing JSON file for get_estimated_time for MPH #467

Open
wants to merge 64 commits into
base: main
Choose a base branch
from

Conversation

alex-l-kong
Copy link
Contributor

What is the purpose of this PR?

Closes #465. The way list_files works is not robust against the new _processing.json files that get written to the CACs. For get_estimated_time, this causes issues when verifying that a FOV exists in the run folder, then loading it in because sometimes it will attempt to verify the _processing.json file and not the main FOV .json file.

How did you implement your changes

Make use of exact_match=True for list_files to enforce hard matching.

Remaining issues

Across several places in toffy and ark, exact_match=True must be used, especially because Bryan's ezSegmenter update makes soft matching the default.

@alex-l-kong alex-l-kong self-assigned this Aug 22, 2024
@alex-l-kong
Copy link
Contributor Author

The current way list_files is implemented for exact_match=True is awkward because it removes file extensions when considering matches: https://github.com/angelolab/alpineer/blob/main/src/alpineer/io_utils.py#L78-L83. Worth thinking if a rewrite is necessary.

Copy link
Contributor

@camisowers camisowers left a comment

Choose a reason for hiding this comment

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

I agree, the logic for dropping the extension when checking for an exact match is probably a good idea. We would just need to check ark and toffy to make sure that isn't in there for a specific reason.

I would add a -processing.json file to all relevant tests in the pipeline, to see if list_files() breaks anywhere else in toffy.

src/toffy/mph_comp.py Outdated Show resolved Hide resolved
alex-l-kong and others added 28 commits November 12, 2024 14:25
…d TODO for fixing verify_same_elements edge case
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.

Script 3d sometimes tries to read the wrong json file
3 participants