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

FOV resolution check #391

Merged
merged 9 commits into from
Jul 19, 2023
Merged

FOV resolution check #391

merged 9 commits into from
Jul 19, 2023

Conversation

camisowers
Copy link
Contributor

@camisowers camisowers commented Jul 17, 2023

If you haven't already, please read through our contributing guidelines before opening your PR

What is the purpose of this PR?

Closes #388. Calculates the image resolution for each FOV using frameSizePixels[width]/fovSizeMicrons and outputs them for the user to check.

How did you implement your changes

Add a function check_fov_resoltutions() which retrieves the information from the run file and calculated the resolution for each FOV.

Print resolutions in the notebook and have a save_path arg if the user would like to specify a file to save to.
Example notebook output:

Screenshot 2023-07-19 at 9 52 37 AM

Remaining issues

  • Should we leave this as its own notebook or throw the check in somewhere in the pipeline?

@camisowers camisowers added the enhancement New feature or request label Jul 17, 2023
@camisowers camisowers self-assigned this Jul 17, 2023
@review-notebook-app
Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

Copy link
Contributor

@srivarra srivarra 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 a couple usability questions!

src/toffy/json_utils.py Show resolved Hide resolved
src/toffy/json_utils.py Show resolved Hide resolved
Copy link
Contributor

@alex-l-kong alex-l-kong left a comment

Choose a reason for hiding this comment

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

I'd say it's better to put this FOV resolution check directly after the 3a and 3b notebooks, since we'd want to verify that the resolution is correct right after image extraction.

If you're really feeling it, you could potentially even add it to the fov_watcher (full pipeline) and bin_extraction (just extraction) as well.

src/toffy/json_utils.py Outdated Show resolved Hide resolved
@camisowers
Copy link
Contributor Author

camisowers commented Jul 18, 2023

I'd say it's better to put this FOV resolution check directly after the 3a and 3b notebooks, since we'd want to verify that the resolution is correct right after image extraction.

If you're really feeling it, you could potentially even add it to the fov_watcher (full pipeline) and bin_extraction (just extraction) as well.

Agreed, we'll discuss at pipeline today.

Copy link
Member

@ngreenwald ngreenwald 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, one addition

src/toffy/json_utils.py Outdated Show resolved Hide resolved
@ngreenwald ngreenwald merged commit ce36dec into main Jul 19, 2023
4 checks passed
@ngreenwald ngreenwald deleted the fov_resolution_check branch July 19, 2023 19:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Identify resolution per image
4 participants