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

General comments, text and documentation+ curiosity #69

Closed
theanega opened this issue Mar 26, 2024 · 2 comments · Fixed by #70
Closed

General comments, text and documentation+ curiosity #69

theanega opened this issue Mar 26, 2024 · 2 comments · Fixed by #70

Comments

@theanega
Copy link

  1. General - great job and thanks!
    First of all, huge congrats for the quality of the submitted project and for advancing radiomics research. I mean this not only with the MIRP package but also IBSI, IBSI2, etc. I always enjoy your papers. As a potential future user, I particularly appreciate the implementation of the fixed_bin_size_puradiomics feature. By extending MIRP to support non-IBSI compliant implementations, you are enabling other users to use MIRP to validate a wider range of radiomics results. This was a necessary and valuable addition to the field.

  2. Text - minor correction:
    Second paragraph of the Statement of need has 4 in-text citations that do not follow the same format as the others.

  3. Documentation - good to go for now but some areas could benefit from extended content/additional clarifications and examples in the future...

  • Welcome page could be made more engaging and inviting to attract new users. For instance, you could add the statement of need, a section highlighting key features of MIRP and a table of contents. Even a brief section of "what is radiomics?", which could help make the documentation more compelling. Some documentation packages seem to have put a lot of effort into creating an attractive and informative welcome experience and I feel like this benefits adoption. Example (perhaps a bit too much but really fun): https://fusilli.readthedocs.io/en/latest/introduction.html,over 100 stars in a couple of weeks.

  • I would add an installation page "how to install"

  • Would add a quick start script similar to https://fusilli.readthedocs.io/en/latest/quick_start.html

  • Image Processing Settings

    • The note regarding intensity normalisation could be improved, for instance: Intensity normalisation is an important preprocessing step that can help standardize the intensity values across different images or modalities. However, it is important to note that intensity normalisation can remove the physical meaning of the intensity units (i.e. HU values for CT). (Optional: Please see (maybe add a reference here) for more information on the appropriate use and implications of intensity normalisation in medical imaging analysis.)
  • Feature computation settings:

    • Would add somewhere the link to the definitions of features. Sorry if I missed this.
  • Image interpolation settings:

    • I would make the comment of the voxel spacing being smaller in the original image that in the resamples as a proper note so it's more visible.
  • Image transformation settings:

  • Process image and compute quantitative image features

    • Examples could be clarified by listing them and/or adding headings (i.e.):
        1. Computing features from ROIs in original images
        • Without interpolation
        • with interpolation
        • and so on
        1. Computing features from ROIs in filtered images
    • Examples with extract_features_and_images() could be provided
  • Add also the guide for contributors (currently only in gitub repo)

  • In the future I would also add "release notes" and FAQs pages.

  • I would include somewhere the lessons learned from Appendix S8 of the IBSI2 paper (https://doi.org/10.1148/radiol.231319) in the MIRP documentation. These lessons seem highly relevant to the radiomics community, and their current limited accessibility could be improved by including them.

Curiosity/question:

  • Does MIRP allow for computation of radiomics maps? (i.e. output is a 3D image where each voxel has a value per feature?) From my understanding the answer is no and I think that the addition of voxel-wise radiomics maps could be a valuable enhancement to MIRP.  In the same way that spatial transcriptomics provides more information than bulk transcriptomics, I feel like radiomics maps providing values per voxel for specific texture features provide more information than "1D radiomics" (i.e. one value per ROI), what do you think?
@alexzwanenburg
Copy link
Member

Thanks! I have started working on installation and quick-start sections of the long-form documentation.

Regarding your question: MIRP currently does not allow for computing radiomics feature maps. I agree it would be interesting. Creating an implementation that performs well will likely take some time and may require translating parts of the code to C or Cython. It is not something I plan to do anytime soon.

alexzwanenburg added a commit that referenced this issue Apr 2, 2024
alexzwanenburg added a commit that referenced this issue Apr 3, 2024
@Matthew-Jennings
Copy link
Contributor

Matthew-Jennings commented Apr 4, 2024

@theanega and @alexzwanenburg, just on point 2:

I believe that the four citations @theanega refers to are narrative in-text citations whereas the others are parenthetical. It is common to include both formats in a paper. See this APA Style and Grammar Guidelines webpage. I don't think any correction is needed here.

alexzwanenburg added a commit that referenced this issue Apr 5, 2024
Aim is to transition quick_start.rst into the notebook and create a working example.
@alexzwanenburg alexzwanenburg linked a pull request Apr 11, 2024 that will close this issue
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 a pull request may close this issue.

3 participants