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

MAINT: Use PyBIDS 0.9.2+ #369

Merged
merged 20 commits into from
Jul 12, 2019
Merged

MAINT: Use PyBIDS 0.9.2+ #369

merged 20 commits into from
Jul 12, 2019

Conversation

effigies
Copy link
Member

@effigies effigies commented Jul 9, 2019

@effigies
Copy link
Member Author

effigies commented Jul 9, 2019

I think this will require a 0.10.0 release to avoid version conflicts with things expecting pybids 0.7.x, templateflow 0.3.x, and niworkflows 0.9.x.

@effigies
Copy link
Member Author

effigies commented Jul 9, 2019

It's not showing up yet, but I'm moving us to the PEP 440 Compatible Release syntax:

templateflow ~= 0.4.0  # equivalent to...
templateflow >= 0.4.0, == 0.4.*

@effigies
Copy link
Member Author

effigies commented Jul 9, 2019

@oesteban The pure Python bits (I didn't build the container) test clean locally. Most of the changes are in the reports bit, so you should definitely have a look at that.

@effigies
Copy link
Member Author

effigies commented Jul 9, 2019

Ah. Missed utils. Looks like there are some knock-on problems in templateflow, but I don't really know how to diagnose this:

_____________ [doctest] niworkflows.utils.misc.get_template_specs ______________
013 
014     Parse template specifications
015 
016     >>> get_template_specs('MNI152NLin2009cAsym', {'suffix': 'T1w'})[1]
UNEXPECTED EXCEPTION: RuntimeError('Could not find template "MNI152NLin2009cAsym" with specs={\'suffix\': \'T1w\', \'desc\': None, \'atlas\': None, \'resolution\': 1}. Please revise your template argument.')
Traceback (most recent call last):

  File "/usr/local/miniconda/lib/python3.7/doctest.py", line 1329, in __run
    compileflags, 1), test.globs)

  File "<doctest niworkflows.utils.misc.get_template_specs[0]>", line 1, in <module>

  File "/src/niworkflows/niworkflows/utils/misc.py", line 54, in get_template_specs
    argument.""".format(in_template, template_spec))

RuntimeError: Could not find template "MNI152NLin2009cAsym" with specs={'suffix': 'T1w', 'desc': None, 'atlas': None, 'resolution': 1}. Please revise your template argument.

/src/niworkflows/niworkflows/utils/misc.py:16: UnexpectedException

@effigies
Copy link
Member Author

effigies commented Jul 9, 2019

Ah, looks like it's a bug in pybids. bids-standard/pybids#458

@effigies
Copy link
Member Author

Okay, so I think I've fixed the bug in pybids... Now getting this one:

022     >>> get_template_specs('MNIInfant', {'res': '2', 'cohort': '10', 'suffix': 'T1w'})[1]
UNEXPECTED EXCEPTION: RuntimeError('Could not find template "MNIInfant" with specs={\'cohort\': \'10\', \'suffix\': \'T1w\', \'desc\': None, \'atlas\': None, \'resolution\': \'2\'}. Please revise your template argument.',)
Traceback (most recent call last):

  File "/anaconda3/lib/python3.6/doctest.py", line 1330, in __run
    compileflags, 1), test.globs)

  File "<doctest niworkflows.utils.misc.get_template_specs[2]>", line 1, in <module>

  File "/Users/markiewicz/Projects/crn/niworkflows/niworkflows/utils/misc.py", line 54, in get_template_specs
    argument.""".format(in_template, template_spec))

RuntimeError: Could not find template "MNIInfant" with specs={'cohort': '10', 'suffix': 'T1w', 'desc': None, 'atlas': None, 'resolution': '2'}. Please revise your template argument.

/Users/markiewicz/Projects/crn/niworkflows/niworkflows/utils/misc.py:22: UnexpectedException

I think it's because MNIInfant doesn't get installed anywhere, which makes me wonder why it's not failing on Circle.

@oesteban
Copy link
Member

Probably removing the templateflow folder will refresh the installation with the latest scaffold.

Dockerfile Outdated
python -c "from templateflow import api as tfapi; \
tfapi.get('MNI152Lin|MNI152NLin2009cAsym|OASIS30ANTs', suffix='T1w'); \
tfapi.get('MNI152Lin|MNI152NLin2009cAsym|OASIS30ANTs', desc='brain', suffix='mask'); \
tfapi.get('OASIS30ANTs', resolution=1, desc='4', suffix='dseg'); \
tfapi.get('OASIS30ANTs|NKI', resolution=1, label='brain', suffix='probseg'); \
tfapi.get('OASIS30ANTs|NKI', resolution=1, desc='BrainCerebellumRegistration', suffix='mask'); "
tfapi.get('OASIS30ANTs|NKI', resolution=1, desc='BrainCerebellumRegistration', suffix='mask'); " && \
deactivate && rm -rf /tmp/venv
Copy link
Member Author

Choose a reason for hiding this comment

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

pybids was getting installed with templateflow, blocking its upgrade with requirements.txt.

@effigies
Copy link
Member Author

New issue:

________________________________ test_ROIsPlot _________________________________
[gw1] linux -- Python 3.7.1 /usr/local/miniconda/bin/python

    def test_ROIsPlot():
        """ the BET report capable test """
        import nibabel as nb
        import numpy as np
    
        im = nb.load(str(
            get_template('OASIS30ANTs', resolution=1, desc='4',
>                        suffix='dseg', extension=['.nii', '.nii.gz'])))

/src/niworkflows/niworkflows/tests/test_segmentation.py:42: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 

filename = "[PosixPath('/home/niworkflows/.cache/templateflow/tpl-OASIS30ANTs/tpl-OASIS30ANTs_res-01_desc-4_dseg.nii.gz'), PosixPath('/home/niworkflows/.cache/templateflow/tpl-OASIS30ANTs/tpl-OASIS30ANTs_res-01_desc-4_dseg.tsv')]"
kwargs = {}

Can't debug locally because I can't get templateflow configured properly. I can try the Docker image, but this is very annoying.

@effigies
Copy link
Member Author

Turns out it was templateflow/python-client#16.

@oesteban
Copy link
Member

Can't debug locally because I can't get templateflow configured properly. I can try the Docker image, but this is very annoying.

I agree. The most urgent thing, I believe, is making sure that updating TemplateFlow updates the HOME directory at installation (templateflow/python-client#11). Within the context of #374 we can address the problems of testing niworkflows. WDYT?

@effigies
Copy link
Member Author

Sounds reasonable.

@effigies
Copy link
Member Author

_____________ [doctest] niworkflows.utils.misc.get_template_specs ______________
013 
014     Parse template specifications
015 
016     >>> get_template_specs('MNI152NLin2009cAsym', {'suffix': 'T1w'})[1]
017     {'resolution': 1}
018 
019     >>> get_template_specs('MNI152NLin2009cAsym', {'res': '2', 'suffix': 'T1w'})[1]
020     {'resolution': '2'}
021 
022     >>> get_template_specs('MNIInfant', {'res': '2', 'cohort': '10', 'suffix': 'T1w'})[1]
UNEXPECTED EXCEPTION: RuntimeError('Could not find template "MNIInfant" with specs={\'cohort\': \'10\', \'suffix\': \'T1w\', \'desc\': None, \'atlas\': None, \'resolution\': \'2\'}. Please revise your template argument.')
Traceback (most recent call last):

  File "/usr/local/miniconda/lib/python3.7/doctest.py", line 1329, in __run
    compileflags, 1), test.globs)

  File "<doctest niworkflows.utils.misc.get_template_specs[2]>", line 1, in <module>

  File "/src/niworkflows/niworkflows/utils/misc.py", line 54, in get_template_specs
    argument.""".format(in_template, template_spec))

RuntimeError: Could not find template "MNIInfant" with specs={'cohort': '10', 'suffix': 'T1w', 'desc': None, 'atlas': None, 'resolution': '2'}. Please revise your template argument.

/src/niworkflows/niworkflows/utils/misc.py:22: UnexpectedException

Now we're getting the MNIInfant thing on Circle, which is at least vaguely relieving because it's not fetched anywhere. It's unclear why it ever worked.

@effigies
Copy link
Member Author

@oesteban Now I can't replicate the failure locally. Is it just stochastic? I can try restarting, but it's a long wait to find out.

@oesteban
Copy link
Member

It is very weird because a few lines above it says it is downloading https://templateflow.s3.amazonaws.com/tpl-MNI152NLin2009cAsym/tpl-MNI152NLin2009cAsym_res-01_T1w.nii.gz which should be already cached in the image.

There needs to be an obscure way that propagates some older templateflow folder.

@effigies
Copy link
Member Author

Hmm. It repeated, though the Docker image was identical. Do you see any ways forward?

@oesteban
Copy link
Member

oesteban commented Jul 10, 2019 via email

@effigies
Copy link
Member Author

Probably just easiest to ask: How would you identify the templateflow home and the status of the files?

@effigies
Copy link
Member Author

oot@607307af2c0a:/tmp# python -c 'import templateflow.api as tfapi; print(tfapi.TF_LAYOUT.root)'
/home/niworkflows/.cache/templateflow
root@607307af2c0a:/tmp# ls /home/niworkflows/.cache/templateflow/
tpl-MNI152Lin  tpl-MNI152NLin2009cAsym  tpl-NKI  tpl-OASIS30ANTs  tpl-PNC  tpl-fsLR  tpl-fsaverage
root@607307af2c0a:/tmp# ls -AFl /home/niworkflows/.cache/templateflow/
total 28
drwxr-xr-x 3 root root 4096 Jul 10 18:39 tpl-MNI152Lin/
drwxr-xr-x 2 root root 4096 Jul 10 18:39 tpl-MNI152NLin2009cAsym/
drwxr-xr-x 3 root root 4096 Jul 10 18:39 tpl-NKI/
drwxr-xr-x 2 root root 4096 Jul 10 18:39 tpl-OASIS30ANTs/
drwxr-xr-x 2 root root 4096 Jul 10 18:39 tpl-PNC/
drwxr-xr-x 2 root root 4096 Jul 10 18:39 tpl-fsLR/
drwxr-xr-x 2 root root 4096 Jul 10 18:39 tpl-fsaverage/
root@607307af2c0a:/tmp# mv /home/niworkflows/.cache/templateflow{,.old} 
root@607307af2c0a:/tmp# python -c 'import templateflow.api as tfapi; tfapi.get("MNIInfant")'
root@607307af2c0a:/tmp# ls /home/niworkflows/.cache/templateflow
tpl-MNI152Lin  tpl-MNI152NLin2009cAsym  tpl-NKI  tpl-OASIS30ANTs  tpl-PNC  tpl-fsLR  tpl-fsaverage

Maybe MNIInfant isn't always available via the S3 method?

@oesteban
Copy link
Member

Sorry for the slow response. I believe I know what is happening. Please give me a second.

@oesteban
Copy link
Member

Okay, yes.

When TemplateFlow is not using DataLad, the first time the api is called the following happens:

  1. A file like https://github.com/templateflow/python-client/blob/master/templateflow/conf/templateflow-skel.zip is unzipped to the templateflow home. This file contains the skeleton of the templateflow repo, with empty files instead of niftis or giftis. As the files are there, pybids can then query.

  2. Selected files by a query get pulled down from S3 if the selected file size is zero bytes. Empty files are replaced by the freshly downloaded.

  3. Subsequent queries to the same files will end up finding the newly fetched version.

What is happening?

Updating the skeleton file is part of the packaging process. The problem is that, at this moment, the circleci build does not push back the new skeleton to the repo.

Since you are installing from a commit, a very old skeleton is found in the repo (honestly, if it does not get updated it should go away).

Fastest solution for you: cut an rc1 pre-release - it takes 5 minutes.

@effigies
Copy link
Member Author

Sure, let's do that then. Do you want to or should I?

@oesteban
Copy link
Member

@effigies
Copy link
Member Author

Thanks.

@oesteban
Copy link
Member

Thank you for diving into these frozen waters

Dockerfile Outdated Show resolved Hide resolved
niworkflows/reports/core.py Show resolved Hide resolved
@effigies effigies changed the title MAINT: Use PyBIDS 0.9.x MAINT: Use PyBIDS 0.9.2+ Jul 12, 2019
@codecov
Copy link

codecov bot commented Jul 12, 2019

Codecov Report

❗ No coverage uploaded for pull request base (master@44da844). Click here to learn what that means.
The diff coverage is 75%.

Impacted file tree graph

@@            Coverage Diff            @@
##             master     #369   +/-   ##
=========================================
  Coverage          ?   39.01%           
=========================================
  Files             ?       43           
  Lines             ?     4788           
  Branches          ?        0           
=========================================
  Hits              ?     1868           
  Misses            ?     2920           
  Partials          ?        0
Flag Coverage Δ
#unittests 39.01% <75%> (?)
Impacted Files Coverage Δ
niworkflows/utils/bids.py 88.75% <ø> (ø)
niworkflows/interfaces/bids.py 88.52% <100%> (ø)
niworkflows/reports/core.py 80.98% <66.66%> (ø)

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 44da844...df6f3fc. Read the comment docs.

@codecov
Copy link

codecov bot commented Jul 12, 2019

Codecov Report

❗ No coverage uploaded for pull request base (master@44da844). Click here to learn what that means.
The diff coverage is 75%.

Impacted file tree graph

@@            Coverage Diff            @@
##             master     #369   +/-   ##
=========================================
  Coverage          ?   39.01%           
=========================================
  Files             ?       43           
  Lines             ?     4788           
  Branches          ?        0           
=========================================
  Hits              ?     1868           
  Misses            ?     2920           
  Partials          ?        0
Flag Coverage Δ
#reportlettests 100% <ø> (?)
#unittests 39.01% <75%> (?)
Impacted Files Coverage Δ
niworkflows/utils/bids.py 88.75% <ø> (ø)
niworkflows/interfaces/bids.py 88.52% <100%> (ø)
niworkflows/reports/core.py 80.98% <66.66%> (ø)

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 44da844...df6f3fc. Read the comment docs.

@effigies effigies merged commit 08138b8 into nipreps:master Jul 12, 2019
@effigies effigies deleted the dep/pybids_0.9 branch July 12, 2019 12:14
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.

2 participants