search

oncoray / mirp

Medical Image Radiomics Processor
https://oncoray.github.io/mirp/
European Union Public License 1.2
38 stars 11 forks source link

JOSS review item: Examples #72

Closed Matthew-Jennings closed 1 month ago

Matthew-Jennings commented 3 months ago

Hi @alexzwanenburg. Great stuff putting this project together! Apologies for the delay in review, I hope you find the feedback useful. I'll separate my review items into individual issues. If you address an issue in a PR, please link the issue within the PR or vice versa.

The examples provided - forgive me - add almost nothing to the API documentation. I really think it is worth fleshing these out, and probably much more so than, say, separating out the installation instructions into a separate page from the README. In my experience, users heavily rely on examples to grok a library.

Well written examples, IMHO, should depict near end-to-end usage of MIRP's functionality, using actual input data (mock is okay, even preferred, but a placeholder "path/to/input/file" is usually not) and frequently, explicitly printing or plotting outputs along the way. To get an idea, have a look at this not-even-that-great-example in another repo I help maintain, PyMedPhys.

Presumably the meat of most of your conceivable examples is already present in your test suite?

Jupyter notebooks are excellent for this. If you struggle to integrate Jupyter notebooks into the docs, simply include an example folder of them and link to them from the docs.

Let me know what you think, happy to discuss.

alexzwanenburg commented 3 months ago

Thanks for the pointers! I have added a first tutorial to documentation. Check it out here: https://oncoray.github.io/mirp/tutorial_compute_radiomics_features_mr.html

I am leaving this issue open as more tutorials can and should be added.

alexzwanenburg commented 2 months ago

Reopened for JOSS review.

alexzwanenburg commented 2 months ago

A second tutorial may be found here: https://oncoray.github.io/mirp/tutorial_apply_image_filter.html

Matthew-Jennings commented 1 month ago

Great! Happy to consider the JOSS item related to this issue closed. Feel free to close the actual issue if you like :)