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.

Sign up for GitHub

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

Jump to bottom

added option to also re-insert #179

Closed
wants to merge 8 commits into from
Closed

added option to also re-insert #179

wants to merge 8 commits into from

Conversation

gluneau
Copy link
Contributor

@gluneau gluneau commented Feb 15, 2017 •
edited
Loading

It deletes, the re-inserts

By calling the script with the insertminc argument, the minc will be inserted as well.

tools/deletemincsqlwrapper.pl -profile prod -insertminc

@gluneau gluneau added this to the 17.1 milestone Feb 15, 2017
MounaSafiHarab
MounaSafiHarab reviewed Feb 24, 2017
View reviewed changes
tools/deletemincsqlwrapper.pl Outdated
@@ -8,27 +8,36 @@
use Term::ANSIColor qw(:constants);
use NeuroDB::DBI;

my $profile = "prod";
my $profile = "null_grads"; # "prod";
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should not be hard coded; so maybe separate into 2 pull requests

MounaSafiHarab
MounaSafiHarab reviewed May 30, 2017
View reviewed changes
tools/deletemincsqlwrapper.pl Outdated
or (pf.ParameterTypeID=333 and pf.`VALUE` like '%0., 0., 0., 0., 0., 0., 0., 0., 0., 0., 0., 0., 0., 0., 0., 0., 0., 0., 0., 0., 0., 0., 0., 0., 0., 0., 0., 0., 0., 0., 0., 0., 0., 0., 0., 0., 0., 0., 0., 0., 0., 0., 0., 0., 0., 0., 0., 0., 0., 0., 0., 0., 0., 0., 0., 0., 0., 0., 0., 0., 0., 0., 0., 0., 0., 0., 0., 0., 0., 0., 0., 0., 0., 0., 0.%')
or (pf.ParameterTypeID=334 and pf.`VALUE` like '%0., 0., 0., 0., 0., 0., 0., 0., 0., 0., 0., 0., 0., 0., 0., 0., 0., 0., 0., 0., 0., 0., 0., 0., 0., 0., 0., 0., 0., 0., 0., 0., 0., 0., 0., 0., 0., 0., 0., 0., 0., 0., 0., 0., 0., 0., 0., 0., 0., 0., 0., 0., 0., 0., 0., 0., 0., 0., 0., 0., 0., 0., 0., 0., 0., 0., 0., 0., 0., 0., 0., 0., 0., 0., 0.%')
or (pf.ParameterTypeID=343 and pf.`VALUE` like '%0., 0., 0., 0., 0., 0., 0., 0., 0., 0., 0., 0., 0., 0., 0., 0., 0., 0., 0., 0., 0., 0., 0., 0., 0., 0., 0., 0., 0., 0., 0., 0., 0., 0., 0., 0., 0., 0., 0., 0., 0., 0., 0., 0., 0., 0., 0., 0., 0., 0., 0., 0., 0., 0., 0., 0., 0., 0., 0., 0., 0., 0., 0., 0., 0., 0., 0., 0., 0., 0., 0., 0., 0., 0., 0.%'))
ORDER BY f.FileID
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@gluneau

all these hard coded parameter TypeID won't work for other projects; same for SeriesUID

MounaSafiHarab
MounaSafiHarab suggested changes May 30, 2017
View reviewed changes
Copy link
Contributor

@MounaSafiHarab MounaSafiHarab left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@gluneau

  1. hardcoded parameter type IDs and seriesUID

  2. a script description/usage would be helpful since you are adding an argument (it is not very clear how to run the script)

  3. since there is an option to insert deleted files, I am not sure the filename deletemincsqlwrapper.pl is still accurate.

@gluneau
Copy link
Contributor Author

gluneau commented May 30, 2017

@MounaSafiHarab Please check the requested changes.

@MounaSafiHarab
Copy link
Contributor

MounaSafiHarab commented Aug 1, 2017 •
edited
Loading

  1. Mouna to test it
  2. eventually, make this more generic for all projects to be able to use (query is IBIS specific now)

@MounaSafiHarab
Copy link
Contributor

@gluneau

I tested it and I ran into a couple of issues:

  1. I had to update the help section and added the use Getopt::Tabular and my @opt_table sections to reflect the usage of the -insertminc option (you can then make the prod file name an argument as well so it is not hard coded)
  2. More importantly, the minc_deletion, when it is called, is not given the tarchiveLocation as an argument, so tarchiveLoader does not run.

MounaSafiHarab
MounaSafiHarab reviewed Aug 21, 2017
View reviewed changes
tools/deletemincsqlwrapper.pl Outdated
@@ -69,5 +76,11 @@
my $minc_delete_log = `$minc_delete_cmd`;
print $minc_delete_log . "\n";

if ($insertminc) {
my $tar_loader_cmd = "../uploadNeuroDB/tarchiveLoader -profile " . $profile . " -seriesuid " . $fF->{'SeriesUID'} . " -verbose -globLocation " . $fF->{'ArchiveLocation'};
print $tar_loader_cmd . "\n";
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@gluneau
not sure where the ArchiveLocation is coming from. You need to join on the tarchive table to get this location.
(running this with the insertminc option gives the error:

...
usage: ../uploadNeuroDB/tarchiveLoader </path/to/DICOM-tarchive> [options]
       ../uploadNeuroDB/tarchiveLoader -help to list options


	ERROR: You must specify a valid tarchive and an existing profile.

@MounaSafiHarab MounaSafiHarab mentioned this pull request Sep 5, 2017
Merged
@samirdas samirdas closed this Oct 2, 2017
@MounaSafiHarab
Copy link
Contributor

closed but same changes were merged under this pull request:
#193

@cmadjar cmadjar added this to the N/A milestone Feb 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@MounaSafiHarab MounaSafiHarab MounaSafiHarab approved these changes

Assignees

@MounaSafiHarab MounaSafiHarab

Labels
New Feature
Projects
None yet
Milestone
N/A
Development

Successfully merging this pull request may close these issues.
4 participants
@gluneau @MounaSafiHarab @samirdas @cmadjar