[Documentation] NeuroDB::MRIProcessingUtility library perldocification #225

cmadjar · 2017-12-19T23:46:08Z

Using perldoc/perlpod to document MRIProcessingUtility.pm so that users can type in the terminal
perldoc MRIProcessingUtility.pm and get the documentation.

Using the pod2markdown tool from perl Pod::Markdown module, the MRIProcessingUtility.md file has been created so that we have a web displayed version of the documentation.
pod2markdown MRIProcessingUtility.pm > MRIProcessingUtility.md

Dependencies added: perldoc, Pod::Markdown, Pod::Usage

MounaSafiHarab · 2018-01-24T19:16:34Z

uploadNeuroDB/NeuroDB/MRIProcessingUtility.pm

+Writes error log. This is a useful function that will close the log and write
+error messages in case of abnormal program termination.
+
+INPUT: notification message, fail status of the process, log directory


INPUTS instead of INPUT (although this is consistent in the rest of the script, but I believe the previous ones we were using INPUTS)

MounaSafiHarab · 2018-01-24T19:34:45Z

uploadNeuroDB/NeuroDB/MRIProcessingUtility.pm

+
+=head3 getFileNamesfromSeriesUID($seriesuid, @alltarfiles)
+
+?????????????????????????????


This function extracts from the tarchive_files table a list of all the DICOM files with a given seriesUID. This is useful function used spcifically when running tarchiveLoader on a a partial subset of the complete DICOM tar, with the partial subset defined by the optional seriesUID argument (FYI @cmadjar; details here: #138).

MounaSafiHarab · 2018-01-24T19:35:55Z

uploadNeuroDB/NeuroDB/MRIProcessingUtility.pm

+
+INPUT: seriesUID, list of tar files
+
+RETURNS: ???????????????????


Returns a list of the filenames with a given seriesUID (to be later tarred by the extract_tarchive method)

MounaSafiHarab · 2018-01-24T19:36:47Z

uploadNeuroDB/NeuroDB/MRIProcessingUtility.pm

+
+=pod
+
+=head3 new($dbhr, $debug, $TmpDir, $logfile, $verbose) (constructor)


maybe use >> constructor (just for consistency)

MounaSafiHarab · 2018-01-24T19:42:21Z

uploadNeuroDB/NeuroDB/MRIProcessingUtility.pm

+
+Gets the upload ID form the mri_upload table using tarchive's SourceLocation
+
+INPUT: tarchive's source location


I always get confused when we say tarchive (and in this context it is meant to be the DICOM archive not the tarchive table)

totally agree! Should have said DICOM archive instead

MounaSafiHarab · 2018-01-24T19:42:54Z

uploadNeuroDB/NeuroDB/MRIProcessingUtility.pm

+=head1 TO DO
+
+Document the following functions:
+  - getFileNamesfromSeriesUID($seriesuid, @alltarfiles)


if you use the description above (feel free to reword), then you need to remove this line here from the TODO list

MounaSafiHarab · 2018-01-24T19:44:28Z

uploadNeuroDB/NeuroDB/MRIProcessingUtility.pm

+
+=head3 computeSNR($tarchiveID, $tarchive_srcloc, $profile)
+
+Computes the SNR on the modalities specified in the Config table for a given


specified in the getSNRModalities() routine of the $profile file

oups... I thought it was in the Config table but confused it with Imaging Browser Tabulated Scan Types.

I just looked at the function and wonder if we could not have that in the Config table instead of a function in the prod file. I think I'll create a ticket on redmine to see if there are any other functions that could be moved to the Config table. I think most of them could be removed from the prod file and put into the code base with the regex they use grepped from the Config module.

I think it would be great! but again, same comment on the regex I put on your other Loris pull request; we should avoid regex if we can and use the Config way of adding using dropdowns (like we do for double data entry instruments or tabulated scan types in imaging browser). In this case, it should be done this way as the modalities you want to pick from are already in the mri_scan_type table.
just my opinion!

even better :)

MounaSafiHarab · 2018-01-24T19:46:09Z

uploadNeuroDB/NeuroDB/MRIProcessingUtility.pm

-  $tarchive_srcloc: The Tarchive SourceLocation
+=head3 getUploadIDUsingTarchiveSrcLoc($tarchive_srcloc)
+
+Gets the upload ID form the mri_upload table using tarchive's SourceLocation


please use C<> for table names mri_upload and tarchive; and the column SourceLocation

MounaSafiHarab · 2018-01-24T19:47:04Z

uploadNeuroDB/NeuroDB/MRIProcessingUtility.pm

+
+Calls the Notify->spool function to log all messages.
+
+INPUT:


INPUTS... (I still would have preferred we kept them all at INPUT to be honest).

MounaSafiHarab · 2018-01-24T19:47:36Z

uploadNeuroDB/NeuroDB/MRIProcessingUtility.pm

+
+INPUT:
+  $message   : message to be logged in the database
+  $error     : if 'Y' it's an error log , 'N' otherwise


just please put the 'N' statement under the 'Y'. We did that for other scripts (and you have it like this for the $verb option) 2 lines below

MounaSafiHarab · 2018-01-24T19:48:46Z

uploadNeuroDB/NeuroDB/MRIProcessingUtility.pm

+protocol in the mri_protocol table based on the mincheader information using
+C<&NeuroDB::MRI::identify_scan_db>. If C<$bypass_extra_file_checks> is
+true, then it will bypass the additional protocol checks from the
+mri_protocol_checks table using C<&extra_file_checks>.


C<> for the table names (same also 3 lines above for the mri_protocol table)

MounaSafiHarab · 2018-01-24T19:51:15Z

uploadNeuroDB/NeuroDB/MRIProcessingUtility.pm

+=head3 get_acqusitions($study_dir, \@acquisitions)
+
+Greps the list of acquisitions in the study directory from array
+C<@acquisitions>. ?????????????????????????????????????


I don;t think this method is used anywhere (fetching acquisitions from a .ACQ extension?)

indeed, I just tried grepping for it in the LORIS-MRI code base and found nothing. Will just say unused function that will need to be removed in another PR.

MounaSafiHarab · 2018-01-24T20:22:22Z

uploadNeuroDB/NeuroDB/MRIProcessingUtility.pm

+
+Creates the tarchive information hashref.
+
+INPUT: tarchive's path, globArchiveLocation option


you mean globrchiveLocation is optional?
I believe for optional inputs, you had a standard way of mentioning them in one of the earlier methods (extract_tarchive())in this file. Maybe you can pick a convention and use it?

not really, this refers to the option globArchiveLocation specified as an argument of several scripts. Tried to make this clearer.

MounaSafiHarab · 2018-01-24T20:23:30Z

uploadNeuroDB/NeuroDB/MRIProcessingUtility.pm

+INPUT: scanner ID, tarchive information hashref, boolean if this step should be
+logged
+
+RETURNS: subject's ID hashref


I would specify that it is a hashref containing CandID, PSCID and Visit Label

MounaSafiHarab · 2018-01-24T20:24:09Z

uploadNeuroDB/NeuroDB/MRIProcessingUtility.pm

+
+Extracts and parses the tarchive.
+
+INPUT: path to the tarchive, source location from the tarchive table,


table in C<> format

MounaSafiHarab · 2018-01-24T20:25:49Z

uploadNeuroDB/NeuroDB/MRIProcessingUtility.pm

+
+  use NeuroDB::ProcessingUtility;
+
+  my $utility = NeuroDB::MRIProcessingUtility->new(


indentation is awkward in the SYNOPSIS section; in case you are want them to look perfect when running perldoc :)

MounaSafiHarab · 2018-01-24T20:26:30Z

@cmadjar
I did a first round of looking at this; I can look again and test once some of these comments are incorporated.

cmadjar · 2018-01-25T14:11:14Z

@MounaSafiHarab Thanks a lot for your comments!! I think I incorporated all of them. Let me know if there are other things to change. Will be happy to.

MounaSafiHarab · 2018-02-08T19:00:09Z

uploadNeuroDB/NeuroDB/MRIProcessingUtility.pm

@@ -846,7 +846,7 @@ sub loadAndCreateObjectFile {

 Renames and moves the MINC file.

-INPUT:
+INPUTS:


these inputs in the .md file looks weird (this function and 2 functions below);
please see screenshot (I think indentation is missing)

FYI: they look perfect if I run the perldoc command in aterminal (just the .md one misses the new lines)

MounaSafiHarab · 2018-02-08T19:01:51Z

uploadNeuroDB/NeuroDB/MRIProcessingUtility.pm

-C<@acquisitions>. ?????????????????????????????????????
-
-INPUT: study directory, array of acquisitions ??????????????????
+UNUSED FUNCTION TO BE CLEANED UP IN ANOTHER PULL REQUEST


maybe just say unused here and in the TODO section, we can say removal of unused utilities/methods (such as get_acquisitions) perhaps?

otherwise, tested and everything else looks good.

cmadjar · 2018-02-08T22:57:46Z

@MounaSafiHarab I think I integrated all your feedback. Thanks!!

MounaSafiHarab · 2018-02-14T19:00:31Z

docs/scripts_md/MRIProcessingUtility.md

+  - concat\_mri($minc\_files)
+  - registerProgs(@toregister)
+
+Remove function get\_acqusitions($study\_dir, \\@acquisitions) that is not used


I think you meant to keep either line 362 or line 366 :)

@MounaSafiHarab Good catch! Should be resolved now. Thanks!

MounaSafiHarab · 2018-02-15T14:11:22Z

all good, merging!

[Documentation] NeuroDB::MRIProcessingUtility library perldocification

752aced

cmadjar added the Perldoc label Dec 19, 2017

cmadjar added 2 commits December 22, 2017 10:49

Fixed misalignment in perldoc

352ad7a

Fixed synopsis not indented to specify it is code

4d429cc

MounaSafiHarab self-assigned this Jan 16, 2018

MounaSafiHarab reviewed Jan 24, 2018

View reviewed changes

integrated Mouna's feedback

d4e5a97

MounaSafiHarab reviewed Feb 8, 2018

View reviewed changes

MounaSafiHarab added the PassedManualTest label Feb 8, 2018

Mouna's feedback

3ec3afb

MounaSafiHarab reviewed Feb 14, 2018

View reviewed changes

Mouna's feedback

7374045

MounaSafiHarab merged commit c797df0 into aces:loris-mri-docs Feb 15, 2018

MounaSafiHarab added the PassedCodeReview label Feb 15, 2018

cmadjar deleted the MRIProcessingUtilityPerldoc branch February 27, 2018 16:09

MounaSafiHarab added this to the 19.2 milestone May 30, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Documentation] NeuroDB::MRIProcessingUtility library perldocification #225

[Documentation] NeuroDB::MRIProcessingUtility library perldocification #225

cmadjar commented Dec 19, 2017

MounaSafiHarab Jan 24, 2018

MounaSafiHarab Jan 24, 2018

MounaSafiHarab Jan 24, 2018

MounaSafiHarab Jan 24, 2018 •

edited

MounaSafiHarab Jan 24, 2018

cmadjar Jan 25, 2018 •

edited

MounaSafiHarab Jan 24, 2018

MounaSafiHarab Jan 24, 2018

cmadjar Jan 25, 2018

MounaSafiHarab Jan 25, 2018 •

edited

cmadjar Jan 25, 2018

MounaSafiHarab Jan 24, 2018

MounaSafiHarab Jan 24, 2018

MounaSafiHarab Jan 24, 2018 •

edited

MounaSafiHarab Jan 24, 2018

MounaSafiHarab Jan 24, 2018 •

edited

cmadjar Jan 25, 2018

MounaSafiHarab Jan 24, 2018 •

edited

cmadjar Jan 25, 2018 •

edited

MounaSafiHarab Jan 24, 2018

MounaSafiHarab Jan 24, 2018

MounaSafiHarab Jan 24, 2018

MounaSafiHarab commented Jan 24, 2018

cmadjar commented Jan 25, 2018

MounaSafiHarab Feb 8, 2018

MounaSafiHarab Feb 8, 2018

MounaSafiHarab Feb 8, 2018

MounaSafiHarab Feb 8, 2018

cmadjar commented Feb 8, 2018

MounaSafiHarab Feb 14, 2018

cmadjar Feb 15, 2018

MounaSafiHarab commented Feb 15, 2018


		=head3 getFileNamesfromSeriesUID($seriesuid, @alltarfiles)

		?????????????????????????????


		INPUT: seriesUID, list of tar files

		RETURNS: ???????????????????


		=pod

		=head3 new($dbhr, $debug, $TmpDir, $logfile, $verbose) (constructor)


		Gets the upload ID form the mri_upload table using tarchive's SourceLocation

		INPUT: tarchive's source location


		=head3 computeSNR($tarchiveID, $tarchive_srcloc, $profile)

		Computes the SNR on the modalities specified in the Config table for a given


		Creates the tarchive information hashref.

		INPUT: tarchive's path, globArchiveLocation option


		Extracts and parses the tarchive.

		INPUT: path to the tarchive, source location from the tarchive table,


		use NeuroDB::ProcessingUtility;

		my $utility = NeuroDB::MRIProcessingUtility->new(

[Documentation] NeuroDB::MRIProcessingUtility library perldocification #225

[Documentation] NeuroDB::MRIProcessingUtility library perldocification #225

Conversation

cmadjar commented Dec 19, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

MounaSafiHarab Jan 24, 2018 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cmadjar Jan 25, 2018 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

MounaSafiHarab Jan 25, 2018 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

MounaSafiHarab Jan 24, 2018 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

MounaSafiHarab Jan 24, 2018 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

MounaSafiHarab Jan 24, 2018 • edited

Choose a reason for hiding this comment

cmadjar Jan 25, 2018 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

MounaSafiHarab commented Jan 24, 2018

cmadjar commented Jan 25, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cmadjar commented Feb 8, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

MounaSafiHarab commented Feb 15, 2018

MounaSafiHarab Jan 24, 2018 •

edited

cmadjar Jan 25, 2018 •

edited

MounaSafiHarab Jan 25, 2018 •

edited

MounaSafiHarab Jan 24, 2018 •

edited

MounaSafiHarab Jan 24, 2018 •

edited

MounaSafiHarab Jan 24, 2018 •

edited

cmadjar Jan 25, 2018 •

edited