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.

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

WiP: ReproIn heuristic - support usecase from @mih et al. #187

Merged
merged 9 commits into from
Apr 24, 2018

Conversation

yarikoptic
Copy link
Member

@yarikoptic yarikoptic commented Apr 9, 2018

  • allow/strip any leading CAPITALS followed by a column with the series/protocol name (see xnat support ReproNim/reproin#14)
  • consider series_description after protocol_name if it was not only empty, but didn't parse according to our spec (thus should "allow" for protocol_name to be a fancy "xnatDownload" as in xnat support ReproNim/reproin#14)

@codecov-io
Copy link

codecov-io commented Apr 9, 2018

Codecov Report

Merging #187 into master will decrease coverage by 0.07%.
The diff coverage is 78.33%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #187      +/-   ##
==========================================
- Coverage   68.33%   68.26%   -0.08%     
==========================================
  Files          30       30              
  Lines        2334     2357      +23     
==========================================
+ Hits         1595     1609      +14     
- Misses        739      748       +9
Impacted Files Coverage Δ
heudiconv/bids.py 81.52% <60%> (-1.92%) ⬇️
heudiconv/convert.py 77.13% <80%> (-0.15%) ⬇️
heudiconv/heuristics/reproin.py 83.7% <82.22%> (-0.51%) ⬇️

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 4eff1a6...41ec485. Read the comment docs.

@yarikoptic yarikoptic changed the title WiP: ReproIn heuristic - support usecase from @mih WiP: ReproIn heuristic - support usecase from @mih et al. Apr 11, 2018
bpoldrack added a commit to bpoldrack/datalad-neuroimaging that referenced this pull request Apr 17, 2018
@mih
Copy link
Member

mih commented Apr 23, 2018

If it wouldn't be 'WiP' I would have merged it in order to get a little further with datalad/datalad-neuroimaging#17

@yarikoptic
Copy link
Member Author

bloody python3 zealots ...

SecretStorage requires Python '>=3.5' but the running Python is 2.7.14

so we could expect lots of fun with deployments of datalad if pip would not become smart to not even consider "new" versions for incompatible python. According to PLOS/allofplos#20 it should be the case for pip >= 9. I think we get 10, and I guess that doesn't feature doesn't work since secret storage does have
python_requires='>=3.5', , so either virtualenv causes older pip to be used somehow (yet to check) or that feature in pip is buggy

@mih
Copy link
Member

mih commented Apr 24, 2018

we could expect lots of fun with deployments of datalad

Mission accomplished: datalad/datalad#2442

@yarikoptic
Copy link
Member Author

ok, tests passed and I hope that this doesn't ruin anything, merging

@yarikoptic yarikoptic merged commit 3cc9c57 into nipy:master Apr 24, 2018
@mih
Copy link
Member

mih commented Apr 27, 2018

FTR: With this PR, but without ContentDate/Time:

WARNING: Failed to get date/time for the content: Dataset does not have attribute 'ContentDate'.
Traceback (most recent call last):
  File "/home/mih/env/datalad3-dev/bin/heudiconv", line 11, in <module>
    load_entry_point('heudiconv', 'console_scripts', 'heudiconv')()
  File "/home/mih/hacking/heudiconv/heudiconv/cli/run.py", line 120, in main
    process_args(args)
  File "/home/mih/hacking/heudiconv/heudiconv/cli/run.py", line 330, in process_args
    overwrite=args.overwrite,)
  File "/home/mih/hacking/heudiconv/heudiconv/convert.py", line 198, in prep_conversion
    overwrite=overwrite,)
  File "/home/mih/hacking/heudiconv/heudiconv/convert.py", line 292, in convert
    save_scans_key(item, bids_outfiles)
  File "/home/mih/hacking/heudiconv/heudiconv/bids.py", line 237, in save_scans_key
    op.join(output_dir, 'sub-{0}{1}_scans.tsv'.format(subj, ses)), rows)
  File "/home/mih/hacking/heudiconv/heudiconv/bids.py", line 270, in add_rows_to_scans_keys_file
    data_rows_sorted = sorted(data_rows, key=lambda x: (x[1], x[0]))
TypeError: '<' not supported between instances of 'NoneType' and 'str'

@mgxd mgxd added this to the 0.5.1 milestone Jun 27, 2018
@yarikoptic yarikoptic deleted the enh-mih-case branch April 30, 2021 12:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants