Closed oesteban closed 7 years ago
Thanks Oscar, for going the extra mile of reading the preprint :) and giving me useful suggestions!.
Just to clarify, I didn't intend to connect this to the validity of preprint in any ways - I referred to the preprint as an example to show how it is used in an active research project. In my mind, there are two separate things - paper would be judged on its scientific and research merit separately. The open sourcing of this reimplementation, as you correctly recognize is to open up this analysis to a broader audience, but also to enable them to add their own new ideas for metrics effortlessly (not just those based on histogram distances). I think the the package needs to be judged on its own merits (not a paper outside its scope). Perhaps I can clarify that in the read me, and that will be enough for the review?
As for verifying this implementation with my older matlab implementation, its a huge task and I expect many differences without even trying, for simple reasons 1) that they are based on different histogram distance toolboxes with different implementations, and 2) this was not aimed to exactly mimic my matlab code bit-by-bit, as I was targeting broader audience (beyong structural & thickness data for example). I did put in a lot of work to write many property tests for verifying metrics in this package are indeed metrics ( check hiwenet/test_medpy.py ).
Does that answer your comments?
Updated the readme clarifying the connection to preprint.
PS: reviews like this are useful, making me think about stuff I myself wouldn't have thought about.
updated the paper.md and README.md as suggested improving the description of target audience
Shared the matlab code here as suggested.
Added more older references as suggested. Given the nature of a JOSS paper, I don't think it is possible to elaborate on the context and scientific scope, without making paper.md too long, and I am not sure if the JOSS paper is meant to serve that purpose (its more about the package and what it does). I think users can always read the cited references (now including those you suggested) to get the context and scope.
Very helpful comments so far Oscar. That should have covered your comments so far? Let me know if you have any other suggestions.
Also, added the link to matlab code in readme.
Hey Oscar, just want to note, the build is failing (as per the badge) because I included python 3+ on CI as well as python 2.7. If you check the sub-builds, you can see it passing tests for 2.7, and failing on python 3+ because of incompatible code in the dependency medpy.
Do my comments answer your questions in this issue Oscar? Let me know if otherwise.
Sorry for the wait on this issue.
Just to clarify, I didn't intend to connect this to the validity of preprint in any ways - I referred to the preprint as an example to show how it is used in an active research project. In my mind, there are two separate things - paper would be judged on its scientific and research merit separately. The open sourcing of this reimplementation, as you correctly recognize is to open up this analysis to a broader audience, but also to enable them to add their own new ideas for metrics effortlessly (not just those based on histogram distances). I think the the package needs to be judged on its own merits (not a paper outside its scope). Perhaps I can clarify that in the read me, and that will be enough for the review?
By linking the pre-print you are implicitly making an statement of need: "this software allows you to do this, as shown in [your preprint]". The main figure is shared in both, which also points in that direction. Your claim "there are two separate things - paper would be judged on its scientific and research merit separately" is correct. Therefore, how preprint and code relate to one another should be explicitly stated whenever you use the preprint to support the software: "This is a re-implementation of [your method], based on the experience of [your preprint]", indicating that "results obtained with this library may differ from what you'd get with the original Matlab code". Also, since you have made your Matlab code public, adding the pointer to the original implementation will be really nice: "Here, you will find the original Matlab code we used in the paper just in case you want to replicate our findings" (a documentation page with the script would be great, I added a comment on this to #7). I presume that you will cite this JOSS publication in your preprint so I totally agree with your view (the two things address different facets of scientific communication) but you need to say clearly how and why they differ (IMHO). Otherwise, one could try to reproduce your results without knowing that they are using a different implementation. All of this is (again IMHO) particularly important for the JOSS' Paper.md.
This leads me to the following point.
updated the paper.md and README.md as suggested improving the description of target audience
I think you that the README (which it is probably good enough as it is now, I'll confirm this point when the review is finished) and the Paper.md should be dissociated as they are very different entities with different purposes (please, reformulate Paper.md).
Added more older references as suggested. Given the nature of a JOSS paper, I don't think it is possible to elaborate on the context and scientific scope, without making paper.md too long, and I am not sure if the JOSS paper is meant to serve that purpose (its more about the package and what it does). I think users can always read the cited references (now including those you suggested) to get the context and scope.
(could you point me to the added references?, sorry I don't see them)
As I introduced before, I think the Paper.md should flow as follows:
The box you already have on top with authors and affiliation
Statement of need, including:
2.1. Problem: people want to calculate connectivity matrices based on correlations of structural and functional data (with a reference to support this, probably He 2007?). [2-3 lines]
2.2. Say the purpose of HiWiNet trying to be exact and precise (IMHO you should detach from the definitions in your preprint and think about what exactly your submission does). Cite your preprint here as the main example of application. Indicate the existence of an original Matlab code used in that reference under the documentation folder. The figure may be useful here (I wasn't proposing to remove it before) [4-5 lines].
2.3. Say why HiWiNet is different (core statement of need) to existing software. I gave some examples like MNE (http://martinos.org/mne/stable/generated/mne.SourceEstimate.html#mne.SourceEstimate.extract_label_time_course) or nilearn (http://nilearn.github.io/connectivity/functional_connectomes.html#extracting-signals-on-a-parcellation). Reference those projects.
[5-10 lines].
I think the following addition goes in the right direction, but a clear sentence describing the functionality of HiWiNet with accuracy is still missing.
The target audience is users of almost all neuroimaging modalities who:
- preprocessed dataset already,
- have some base features extracted (node- or patch-wise, that are native to the given modality) using other packages (metioned above), and
- who would like to analyze network-level (i.e. covariance-type or connectivity) relations among the base features (either in space across the cortex or a relevant domain, or across time).
- This is similar to popular metrics of covariance like Correlation or LedoitWolf, and could be dropped in their place. Do you want to find out how histogram-based method compare to your own ideas?
References: the 4 you have so far (one from 2.1, your preprint from 2.2, and the existing software of 2.3). [8 lines]
Total: 26 lines, not too long.
Hey Oscar, just want to note, the build is failing (as per the badge) because I included python 3+ on CI as well as python 2.7. If you check the sub-builds, you can see it passing tests for 2.7, and failing on python 3+ because of incompatible code in the dependency medpy.
Let's follow up that discussion in #6.
I think the last few commits have addressed these suggestions. I esp. like your suggestion 2.3, added it to paper.md and usage_api.rst. Let me know if you suggest anything else.
Hi Oscar, I believe #8 would fix this and let's centralize this discussion there if that's okay with you. Please reopen this otherwise.
I am now checking the last two checkpoints of the review list:
The statement of need basically replicates the summary in the README file. I think that, for JOSS, a more explicit statement should be made. The target audience is not mentioned in the Summary (of course it can be inferred from the summary and the paper, but I think JOSS wants it to be explicit, not implicit).
Some references from the preprint should be extracted and placed in the references section of Paper.md. At least a reference about the earliest structural connectivity analyses that looked at the cortical thickness (He 2007) and references to the historical methods to extract these networks (there is a table about this in the preprint). It is important to place this contribution in the full scientific scope.
Adding those references will also make it easier to describe the unique contributions of this software.
In my opinion, it is also important to mention that this is a reimplementation of an original code in Matlab. First, I want to applaud the authors' transparency noting this point and their effort of providing a Python version that opens up the audience field. Second, I would advice to include the original Matlab code in the repo, as part of the documentation. That way the authors ensure the transparency of the reimplementation. Third, this point is a bit disturbing to me, and I'm not sure of my opinion on this. Should the Matlab code be the one to be submitted to JOSS?
Additionally, (and this goes beyond this review, it is more for the pre-print), I'd advise to repeat all the analyses done in the paper with this software (if possible), to check that this re-implementation works exactly as the Matlab implementation.
(ref. https://github.com/openjournals/joss-reviews/issues/380)