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

Add output of corrected images to example notebooks #132

Conversation

jenna-tomkinson
Copy link

@jenna-tomkinson jenna-tomkinson commented Aug 2, 2023

Hello!

In this PR, I included the following changes:

  1. Rerun the example notebooks so that the outputs are included in order.
  2. Added comments into the notebooks for more interpretability
  3. Including a last code cell in all notebooks to output the corrected images in the most optimal method (e.g., skimage.io.imsave from issue Saving Corrected Images Example #91)
  4. Editing paths in the main README to these updated notebooks

@jenna-tomkinson
Copy link
Author

@yfukai

I would appreciate it if you could give this pull request a review! I can not add anyone, so I will mention you in this comment. Thank you!

@jenna-tomkinson
Copy link
Author

Hi @yfukai, @tying84, and @Nicholas-Schaub,

I know you all are probably super busy but wanted to give a small poke about this PR.

I am really excited to see what comments you all have 😄.

Thank you in advance!

@yfukai
Copy link
Collaborator

yfukai commented Aug 23, 2023

Hi @jenna-tomkinson, thank you for this pull request, and I'm sorry for the late reply; I myself have been completely involved with the benchmarking for the publication (and also other projects). Right, as you point out, it is more useful if the link is directed to the notebook, including the results.

Actually, those notebooks are automatically executed when building the document ( https://basicpy.readthedocs.io/ ), and I believe that may be better than including the executed results since

  • the notebook results are automatically updated to the latest version whenever we build the document, and we can keep the notebook consistent with what the latest package does.
  • we can keep the repository (slightly) lighter.

About the image saving, we actually don't recommend

  • converting the images back to uint8 or uint16
  • rescaling the images by min and max

since it can lead to information loss (by discretization) or new artifacts. The images can be saved as npy files or TIFF images but in a float format.

With this, I was wondering what you think about

Thanks!

Copy link
Collaborator

@yfukai yfukai left a comment

Choose a reason for hiding this comment

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

May be a better option, thanks!

@jenna-tomkinson
Copy link
Author

@yfukai

Thank you for the review! I did not realize that is why the notebooks were not run, so I am glad to learn something new!

I agree with all of the comments and I can easily implement this!

I will note, I do have competing priorities that I will be directing my attention to, so I hope to get to finishing this PR in the near future. I will be keeping this on my list of to-dos, but I probably won't get back to this for a few weeks. If it seems like it has been a very long time, please feel free to poke me.

Thank you again!

@Nicholas-Schaub
Copy link
Collaborator

@jenna-tomkinson I also apologize for being slow to respond. Do you still intend on completing this?

@jenna-tomkinson
Copy link
Author

Hello @Nicholas-Schaub!

I am still interested in completing this PR, but my bandwidth this month is stretched thin with other priorities.

I will likely come back to finish this up in November.

@jenna-tomkinson
Copy link
Author

Hello all,

My apologies for not getting back to this PR sooner. After considerable thought, I have decided to close this PR until further notice due to other competing priorities.

Thank you for the opportunity, and I wish you all the best of luck with this great tool!

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