[ENH] Add ability to rename xnat sessions by DESm1th · Pull Request #263 · TIGRLab/datman

DESm1th · 2020-02-20T18:34:07Z

This pull request:

Adds functions to datman.xnat.xnat to allow experiments and sessions to be renamed
Adds the ability to get rid of those stupid stuck AutoRun.xml pipelines
Adds a bin script (dm_xnat_rename.py) to rename from the command line and in bulk if given a csv of names.
Fixes a minor bug with archive_manifest.py

still needs to be cleaned up and further tested for edge cases...)

… pha_renaming

pep8speaks · 2020-02-20T18:34:12Z

Hello @DESm1th, Thank you for updating!

Cheers! There are no style issues detected in this Pull Request. 🍻 To test for issues locally, pip install flake8 and then run flake8 datman.

Comment last updated at 2020-04-03 20:21:26 UTC

DESm1th · 2020-02-21T00:52:56Z

Oh no, circleci is broken :(

jerdra · 2020-02-21T04:25:33Z

ahh its building off your configuration of circleci and failing, could you try pulling our circleci commits into your repo?

DESm1th · 2020-03-12T22:19:19Z

Don't review yet, I need to fix some function references tomorrow :)

codecov · 2020-03-25T18:59:59Z

Codecov Report

Merging #263 into master will decrease coverage by 0.46%.
The diff coverage is 3.87%.

@@            Coverage Diff             @@
##           master     #263      +/-   ##
==========================================
- Coverage   31.15%   30.68%   -0.47%     
==========================================
  Files          55       56       +1     
  Lines        8497     8645     +148     
==========================================
+ Hits         2647     2653       +6     
- Misses       5850     5992     +142

Flag	Coverage Δ
#unittests	`30.68% <3.87%> (-0.47%)`	⬇️

Impacted Files	Coverage Δ
bin/archive_manifest.py	`0.00% <0.00%> (ø)`
bin/dm_xnat_rename.py	`0.00% <0.00%> (ø)`
datman/xnat.py	`26.92% <8.95%> (-1.43%)`	⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8708908...9a50b22. Read the comment docs.

DESm1th · 2020-03-25T19:21:16Z

@jerdra, @gabiherman I'm done updating this one now, no rush, but it's now safe to review any time :)

gabiherman · 2020-03-27T17:37:10Z


+    def rename_session(self, project, old_name, new_name, rename_exp=True):
+        """
+        Will rename an existing XNAT session (given as 'old_name') to


Do you think it would be excessive to give a warning that rename_exp is true by default? Might be a good idea, but I'm not 100% sure of the use case

I think you're somehow viewing an outdated view of the PR. This is the current signature for this function (I had also changed the function name):

def rename_subject(self, project, old_name, new_name, rename_exp=False):

gabiherman · 2020-03-27T17:44:04Z

+    names = read_sessions(name_path)
+    for entry in names:
+        try:
+            rename_xnat_session(xnat, entry[0], entry[1], project=project)


also nitpicky and probably not necessary, but would improve readability if this was a dict so you could index by entry.old_name, entry.new_name or whatever. (again please feel free to ignore)

That's a good point! I updated the loop to just unpack the tuple properly. It is a lot more readable that way, thanks for catching that :)

jerdra · 2020-02-24T16:15:25Z

+    Sometimes XNAT renames only the subject or only the experiment when you
+    tell it to rename both. Sometimes it says it failed when it succeeded.


Clarify this statement to indicate that experiments being renamed w/o session also being renamed doesn't happen

I think you got bit by the same outdated copy of dm_xnat_rename that gabi did. This function is completely gone in the most up to date copy

ohhh, its cause i didn't submit my review until now haha, i put that up forever ago 😆

jerdra · 2020-04-03T16:37:24Z

+    headers = arguments['--headers'] and arguments['--headers'].split(',') or \
+        default_headers[:]


Suggested change

headers = arguments['--headers'] and arguments['--headers'].split(',') or \

default_headers[:]

headers = (arguments['--headers'] and arguments['--headers'].split(',')) or \

default_headers[:]

Is this the intended evaluation? Might be clearer with parentheses

jerdra · 2020-04-03T16:54:34Z

+def get_xnat(server, user, password):
+    if not server:
+        config = datman.config.config()
+        server = datman.xnat.get_server(config)
+    if not user or not password:
+        user, password = datman.xnat.get_auth()
+    return datman.xnat.xnat(server, user, password)


why not use datman.xnat.get_connection instead?
Just curious!

I added this before I updated datman.xnat to have get_connection and the copy of datman.xnat on this branch is out of date, ha.

jerdra · 2020-04-03T17:24:07Z

+        if not rename_exp:
+            return
+
+        self.rename_experiment(project, new_name, old_name, new_name)


Suggested change

if not rename_exp:

return

self.rename_experiment(project, new_name, old_name, new_name)

if rename_exp:

self.rename_experiment(project, new_name, old_name, new_name)

return

jerdra · 2020-04-03T20:53:00Z

looks good to me!

DESm1th added 17 commits August 13, 2019 17:08

Added a function to rename sessions + experiments

d39ea7c

Untested first version of general xnat session rename script

d86a875

Started adding script for phantom renaming

04e179f

Merge remote-tracking branch 'upstream/master' into pha_renaming

8a35d53

Added crazy error correction. Script works reliably now (but

da98ed3

still needs to be cleaned up and further tested for edge cases...)

Improved logging and made pep8 changes

fe9085d

Refactored code a bit, change logging a bit

4069840

Started working on a phantom renaming script

301bd43

Updated doc string a bit, added pseudocode main function

dd22551

Updated doc strings and started work on pha_rename

b1ea103

Merge branch 'pha_renaming' of https://github.com/DESm1th/datman into…

0aa7210

… pha_renaming

FIX: arguments were incorrectly being set when --headers not used

f8e7df1

Removed unneeded 'e' var

aa62213

Fixed line lengths, added exception to catch when session not found

86a7a53

Fixed autorun issues + split moved experiment renaming to new method

1b52c40

Flake8 fixes

3b4598f

Removed no longer relevant error handling

b046fe8

auto-assign bot requested review from gabiherman and jerdra February 20, 2020 18:34

Fixed missed archive_manifest flake8 error

87cd28f

DESm1th added 2 commits February 21, 2020 10:26

Merge remote-tracking branch 'origin/master' into pha_renaming

b973821

Merged in xnat changes but need to update function references

b9eee82

DESm1th added 2 commits March 13, 2020 14:20

Updated rename_subject and rename_experiment

f808706

Updated xnat_rename to work with KCNI convention too

fbecb28

gabiherman reviewed Mar 27, 2020

View reviewed changes

Updated entry unpacking at Gabi's suggestion

0c85052

gabiherman previously approved these changes Mar 27, 2020

View reviewed changes

jerdra requested changes Apr 3, 2020

View reviewed changes

DESm1th added 2 commits April 3, 2020 15:47

Merge remote-tracking branch 'upstream/master' into pha_renaming

9465eb4

Jer's suggested changes

9a50b22

DESm1th dismissed gabiherman’s stale review via 9a50b22 April 3, 2020 20:21

jerdra approved these changes Apr 3, 2020

View reviewed changes

gabiherman approved these changes Apr 3, 2020

View reviewed changes

DESm1th merged commit b87170f into TIGRLab:master Apr 3, 2020

		Sometimes XNAT renames only the subject or only the experiment when you
		tell it to rename both. Sometimes it says it failed when it succeeded.

		headers = arguments['--headers'] and arguments['--headers'].split(',') or \
		default_headers[:]

Conversation

DESm1th commented Feb 20, 2020

Uh oh!

pep8speaks commented Feb 20, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Comment last updated at 2020-04-03 20:21:26 UTC

Uh oh!

DESm1th commented Feb 21, 2020

Uh oh!

jerdra commented Feb 21, 2020

Uh oh!

DESm1th commented Mar 12, 2020

Uh oh!

codecov bot commented Mar 25, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

DESm1th commented Mar 25, 2020

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jerdra Apr 3, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jerdra commented Apr 3, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

pep8speaks commented Feb 20, 2020 •

edited

Loading

codecov bot commented Mar 25, 2020 •

edited

Loading

jerdra Apr 3, 2020 •

edited

Loading