-
Notifications
You must be signed in to change notification settings - Fork 3
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
Mph plots #101
Mph plots #101
Conversation
Check out this pull request on See visual diffs & provide feedback on Jupyter Notebooks. Powered by ReviewNB |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It looks like the functionality is all here. However, the goal of the notebooks is to have a clean interface for the user to work with. This means abstracting all of the main code into helper functions that are called. This also allows the code to be re-used by the watcher functions. However, this notebook has everything in individual cells
I need to add some kind of error or warning for if the CSVs already exist. We could just pull the data from those or compute the mphs again to be safe and overwrite with new files. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice job pulling out the logic into functions. Some comments about how to make these functions compatible with the watcher
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Almost there!
The updated watcher notebook was tested against data from creed and ran smoothly. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, just some final cleanup
will update the image extraction notebook name and readme link in PR #130 |
If you haven't already, please read through our contributing guidelines before opening your PR
What is the purpose of this PR?
Closes #87.
Create an example notebook for MPH plots that updates the x-axis to use cumulative total counts for each FOV.
Also is watcher compatible.
Remaining issues
Any formatting fixes or plot design.
The combined csv for all the data has
pulse_heights
andcum_total_count
and stored. If we also want the individualtotal_counts
and/or the FOV number included I can add those as well.