[JOSS review] Article comments #236
Comments
|
ready to go |
The guidelines https://joss.readthedocs.io/en/latest/submitting.html say 'A Statement of need section that clearly illustrates the research purpose of the software and places it in the context of related work'. Therefore, what it does is required. We have added however the 'why' we created it as introductory sentence.
There are no other tools doing that type of conversion for PET data, and mention that in the 'why' introductory sentence.
We could but rather stick to the minimalistic recommended JOSS guideline section format.
The section on conversion was rewritten to make it clear is it done using a single wrapper function for DICOM files or a single wrapper function for Ecat files. We also clarified that there are 3 spreadsheets export functions.
JSON are always sidecar files, we now distinguish the image from tsv sidecar JSON.
Given that we now explain that there are 3 functions, those points are logical - but removed a and b. Inline comments
added ref to DICOM and added working for ECAT is proprietary
not necessarily, frame information is ofen present, so we added 'e.g. time zero' as a different example.
This sentence has been removed.
We now use code library throughout.
Sure - rewritten.
Those are the people who wrote the ecat reading function.
We meant that we tested the reading/writting of the data, i.e. which bits are read according to the PET data frames. Referering now explicitly to ecat validation.
fixed
no 'always' is correct - we have never seen a JSON file with all the metadata simply because information from the scanner do not encode eg pharmaceutical information.
fixed
fixed
changes to Supported formats are .xls, .xlsx, .csv, .tsv and .bld.
The conversion goes the other way around. We have now changed this to BIDS blood.tsv to make it clear. |
|
updated are per other reviewer request as well |
|
I feel that the structure of the article can be better. I would suggest to add a section |
|
that is not the JOSS format - I do not want to change their structure |
|
Paragraph on spreadsheet conversion is still quite heavy, you can rewrite it in much more concise way, espetially that there very little difference between (i) and (ii). One just create JSON, other updates JSON. You can say that pet2bids allow to extract blood data from pre-formatted tabular data (including PMOD one) and supplement the sidecar JSON together with blood.tsv. |
|
Are all technical details in file conversion really needed? As reader, I don't really care that you use dcm2niix or Nibabel. I feel that it can be replaced by how and what kind of metadata user can and must supply. |
|
'Are all technical details in file conversion really needed? As reader, I don't really care that you use dcm2niix or Nibabel. I feel that it can be replaced by how and what kind of metadata user can and must supply.' it does, because we are talking quantitative data - not relative like MRI - that's why we have test counting the rounding errors and stuff like that - so for PET people it is essential IMO |
|
'Paragraph on spreadsheet conversion is still quite heavy, you can rewrite it in much more concise way, espetially that there very little difference between (i) and (ii). One just create JSON, other updates JSON. You can say that pet2bids allow to extract blood data from pre-formatted tabular data (including PMOD one) and supplement the sidecar JSON together with blood.tsv.' sure - done 3e28b32#diff-0403ef06adf405f7b310b4518bd6a3559854f54c61676f676ce9cbfee7172ab6 |
|
There still some vocabulary problems. BIDS is not data format, is a structure. So you can't convert, but organize. Second, there a confusions between command line tools, functions and scripts. Command line tool is something I can use in terminal without calling interpreter. The python version of dcm2niix4pet is a command line (because of main). Function is an object that I can import and use inside scripts, functions and interpreter. Matlab version of dcm2niix4pet is a function. I would mention the CLI only at the end paragraph of L30. |
|
but you convert speadsheet to json, DICOM and ecst to Nifti - so I do not where is the issue in the txt? |
|
The starting phrase of PET metadata is still awkward. The important point is that JSON updator updates json and check for consistency and missing information, everything else is not important. |
|
Some ... information are always missing sounds like a truism ?? ok your previous comment was that surely it is not always |
|
I would mention the CLI only at the end paragraph of L30. I could --> 'Note that the python tools are command line tools, i.e. they can be called directly from a terminal. ' ? |
If you want, but you still need a paragraph providing an overview of what exactly pet2bids do. It may be short, but without it, It will also resolve the issue that you never mention that pet2bids sort dataset into BIDS folder structure. P.S. There was a way to start a tread to keep stuff organized. Can't find it( If you know how to do it, please help |
|
overview of what exactly pet2bids do. -- that what the statement of need does as per JOSSS structure -- 'Here we present a new code library, developed in both Matlab and Python, allowing the conversion of DICOM [@dicm_ref_2020] and ECAT (CTI/Siemens proprietary data format) PET imaging data and metadata (e.g., timing information such as e.g. 'time zero' or blood measurements) into the BIDS specification.' |
|
pet2bids sort dataset into BIDS folder structure. ?? but it does not (hence not 'mentioned') |
I understand the importance of testing. But, why do you not test conversion from DICOM, or conversion from ECAT in matlab? It's not the problem that you mention the tests, but how you do it. |
|
I understand the importance of testing. But, why do you not test conversion from DICOM, or conversion from ECAT in matlab? what ?? conversion form DICOM is tested by dcm2niix - conversion from ecat is tested both matlab and python - I do not get your point |
|
Line 30: vocabulary problem -- BIDS is not format, it's a data structure. You can convert from DICOM to Nifti, but you must organize following BIDS. I know it's annoying, but you should not mislead readers. ??? we do not organize nothing -- we convert that's all -- what sentence is misleading? |
That's right, some tools can organize - none can convert
fine
slightly updated, left wrapper - your opinion the name is 'ugly'
added
I don't think so, the new converter relies on a function that reads
why? there is no convention for that in the JOSS ref system
requested by over reviewer
?? doesn't work as a sentence
idiosyncrasies of the code
fixed
PMOD is a very specific tabular type - what can it not do from others? because there is no standard for that
no it's there - @burger1997 ac4a735#diff-0403ef06adf405f7b310b4518bd6a3559854f54c61676f676ce9cbfee7172ab6 |
Exactly the point. BIDS imposes folder structure, specific naming, specific metadata and specific format. When you tell that you convert to BIDS, it is understood that you do everything. If you do only format and metadata, it must be told explicitly. And probably motivate why you don't do folder structure (which is the easiest part of bidsification). For example you can say that the goal of your library is to manage the complex BIDS-PET metadata, and it is supposed to be used from inside of other bidsifiers like ezBIDS and BIDSCOIN. |
|
got you now, makes sense --> c026ad4#diff-0403ef06adf405f7b310b4518bd6a3559854f54c61676f676ce9cbfee7172ab6 (made a commit on top to fix typos) |
It's not what you write, it's how you write.
I'm still not sure what exactly you wanted to say, but your point is that because ecat2nii is home made function, we did additional tests to ensure the quality of conversion. It could look something like: In order to insure the quality of conversion (especially important in quantitative imagery, like PET), we developed a python tool that compares the original ECAT image with converted one. |
|
again, got you know -- again makes sense! |
bidsme can convert, it's not difficult to use dcm2niix (at least for DICOM). Your strong point is metadata.
What was ugly it's not
Just don't like the word
There a convention of not ugly nicknames) It looks very weird inside the text. Also, if BT are the initials (that I'm not sure), the rules of English applies. If BT stands for something else, then the initials of guys must be retrieved (can be not easy).
Got what did you meant. I still don't like the
I would say that you should fix it, but I also understand why you will not)
Just for me: If one have PMOD file, then you can extract blood/plasma/parent fraction data,and corresponding json But if one have just a table, pet2bids can only complete the image sidecar json? So what the point to parse tabular data with spreadsheet convertion, if one just can load his table and use update_json_pet_file?
Yes, saw it) One small thing, in generated pdf, the underscores looks very long, can you try to escape one, just to see if printing will improve? For ex. replace I'll check the changes you made later. Need to work at least a little today. Cheers, |
|
1 - blood data are unusual and do not come with all imaging |
|
Based on draft of c8eaf08. I collected here the points that paper is either misleading or straight incorrect. Line 31:
Misleading, implying that you convert and structure the data following BIDS. I asked several persons and they all reads it this way. To rephrase. Line 32:
Factually incorrect. You provide CLI only for Python, for matlab you provide functions. Line 41:
Factually incorrect. The functions Line 46:
Incomprehensible. To rewrite. Line 55:
Factually incorrect. |
|
[x] 'PET2BIDS is the first tool to convert ... to BIDS' --> made the statement valid --> .. convert to BIDS files. (note we already state above that it's only files and not structure) [x] command line only -- removed only [x] because -- made it correct --> 'to be BIDS valid' [x] functions - yeah, after so many changes requested, the whole paragraph lost its meaning -- rephrase entirely. 'Some radiotracer and pharmaceutical information are always missing in the JSON sidecar files created from reading PET scanner data which is why dcm2niix4pet and ecat2nii take additional metadata. It is also possible to update existing JSON sidecar files with new metadata directly using updatejsonpetfile.m or update_json_pet_file.py. ' |
Can you rewrite full this part? Not adding few words, making phrase even more awkward. In the end of the paragraph, you say the same thing, but in much more concise and clear way: |
|
does really work either -- changed to 'Those functions extend the image sidecar JSON file generated by dcm2niix with metadata from the users that are necessary to make files BIDS-compliant' |
|
Check if it sounds better: Those functions can extend the image sidecar JSON file generated by dcm2niix with user-provided metadata, making them BIDS-compliant. Or: Or even: |
|
sure done |
|
What about the
I propose: The Python code for PET conversion mirrors the Matlab code, except writing of Nifti files, which are delegated to Nibabel [@brett_nipynibabel_2023]. Or something similar. You mention the tests of conversion directly after, so no need |
|
done |
|
Thanks, I'm closing this issue. P.S. Spotted a small error (for sure editor will tell you anyway): on line 19 you forgot a comma after |
Dear pet2bids authors, this is in reference to the review of the paper that I'm currently conducting for JOSS here.
First thanks to put an effort for developing the bidsifier for PET data -- the BIDS specification for PET is quite complex, and your tool is for sure is needed!
I will put my comments in the checklist below:
General comments
Statement of needsdo not state the need of software, you may add a small trivial phrase why pet2bids exists (not what it does).File conversation/PET Metadata/Spreadsheet conversationmay be worth to be put in the dedicated (sub)sectionsJSON filetosidecar JSON file, first one is an any generic JSON file, second is BIDS specific one that supplements image data with metadata.(a),(i), etc.) -- you never reference them. Maybe replace the firstandat line 54 byas well as, and make the iteration in lines 55-58 to a list. I think it would be easier to read.Inline comments
library code, allowing conversion ... using the command line, that you provide a library and a (command line) script?library codereads as a code that do something with library. Library of tools/functions sounds better for me.it can be integrated into softwarebecause it's library, not because it modular. Need to be rephrased.the further testing of data reading?JSON files created from reading PETrefers to JSON sidecar files from conversion using pet2bids.data are always missing...,alwaysseems too strong, maybe better to replace byoftenorusually.original JSON file, I guess you meant sidecar JSON file?PET JSON updater, can you change it into function name(s)?xls, xlsx, csv, tsv, bldit extensive list of tabular data formats in existence, or list of data that bids2pet supports? If it's first, than anfor examplemay be needed.blood.tsvfile, you never mentioned it before. Maybe rephrase it toexports blood data from PMOD tabular file?in our knowledge,to organize. I know at least few tools that can organize PET dataset at least to some extent. Either go withFirst PET dedicated, or stress that your tool do organization in the most complete way.functions which are wrapper functionsis ugly.Thatas it is placed here, refers to dcm2niix, not to dcm2niix4pet. Also it is not clear how the details not from source images are retrieved, are they supplied by user?the newly createdcontradicts their reliance on code of Christian and Muzicand must providetowith user providedupdate_json_pet_filename is different in matlab and in python?update_json_pet_fileis pointless. You already stated that user can provide additional metadata to conversion, and the fact that you describeupdate_json_pet_filemeans that user can call it.Style comments
Here the things that I find not beautiful, but exactly necessary to fix.
suchtools -- sounds better for meThe text was updated successfully, but these errors were encountered: