[MRG] Fix prov and rdflib in nipype

[satra]

this is just a test to get circle feedback on workflows.

[effigies]

@satra I'm assuming this isn't going to make it into 1.1.3?

[satra]

@effigies - when do you want to cut 1.1.3 ?

[effigies]

Monday. (My targets have been the last Mondays of every month.)

[satra]

thanks @effigies - i'm running the test locally to try and replicate the issue. should be able to update this PR with a yay/nay in the next day or two.

[satra]

the good news is that it doesn't crash locally with the special rdflib and the updated prov. i need to figure out how to get the setup procedure install the special rdflib. simply adding it to requirements was insufficient.

this may need to be added to setup.py as well. will see if that works.

[codecov-io]

Codecov Report

Merging #2701 into master will increase coverage by 0.76%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master    #2701      +/-   ##
==========================================
+ Coverage   66.92%   67.68%   +0.76%     
==========================================
  Files         340      340              
  Lines       43144    43364     +220     
  Branches     5349     5448      +99     
==========================================
+ Hits        28876    29353     +477     
+ Misses      13528    13304     -224     
+ Partials      740      707      -33

Flag	Coverage Δ
#smoketests	50.58% <100%> (+1.85%)	⬆️
#unittests	65.17% <100%> (+0.09%)	⬆️

Impacted Files	Coverage Δ
nipype/info.py	94.02% <100%> (ø)	⬆️
nipype/utils/spm_docs.py	67.85% <0%> (-2.52%)	⬇️
nipype/pipeline/plugins/slurm.py	14.73% <0%> (-2.15%)	⬇️
nipype/interfaces/afni/preprocess.py	80.79% <0%> (-0.71%)	⬇️
nipype/pipeline/plugins/legacymultiproc.py	61.3% <0%> (-0.6%)	⬇️
nipype/interfaces/fsl/preprocess.py	82.48% <0%> (-0.12%)	⬇️
nipype/pipeline/engine/utils.py	82.64% <0%> (+0.11%)	⬆️
nipype/algorithms/misc.py	45.97% <0%> (+0.11%)	⬆️
nipype/utils/provenance.py	84.71% <0%> (+0.31%)	⬆️
nipype/interfaces/fsl/utils.py	70.32% <0%> (+0.65%)	⬆️
... and 10 more

---

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 fa0a101...6d3e209. Read the comment docs.

[satra]

@effigies - this passes. if you can take a look at the changes, i would appreciate a notion of how we make this pip installable as a wheel, and enable the corresponding conda changes.

this dependence on a fork of rdflib is super frustrating!

[effigies]

I think the cleanest option would be to give the rdflib fork a PyPI name that it can push wheels to.

For conda, as the feedstock maintainer, you could see about giving it some kind of tag that we can depend on that hopefully would not force-upgrade people happily using 4.2.2.

[satra]

@tgbugs - do you think this would be possible for your rdflib fork?

[tgbugs]

Ya, I can create https://pypi.org/project/neurdflib/ if that works for you all.

[satra]

@tgbugs - let's give it a try.

[tgbugs]

Ok, it's up. I may still have had setup.py in dev mode instead of release mode, so if there are any issues with the rdflib version printing let me know and I can fix it.

[satra]

that installed fine, but i think i can't force it replace rdflib. so i will also need to change the prov package that depends on rdflib! and wherever we import rdflib it would have to be replaced by neurdflib

hmmm - will need to think about a good path forward.

[effigies]

neuprov? :-P

[effigies]

Maybe we need a PEP to support the equivalent of dpkg's "provides".

[tgbugs]

Imports don't have to change. It works with import rdflib if that is what you are worried about.

[satra]

the problem is that prov is dependent on rdflib so when i do a pip install it also installs rdflib.

Successfully installed neurdflib-5.0.0 nipype prov-1.5.2 rdflib-4.2.2

neuro@b1d6b4c7a192:/src$ python
Python 3.6.5 | packaged by conda-forge | (default, Apr  6 2018, 13:39:56) 
[GCC 4.8.2 20140120 (Red Hat 4.8.2-15)] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> import rdflib as rl
INFO:rdflib:RDFLib Version: 4.2.2
>>> 

[tgbugs]

Hrm. The hackish way to fix it is to install prov, uninstall rdflib, and then install neurdflib.

[effigies]

We can get there on a given system, but what we need to work for users is pip install --upgrade nipype or conda update nipype.

[tgbugs]

I guess the question is if there is a way to have the neurdflib package from nipype's requirements.txt get priority for site-packages/rdflib over the rdflib package in prov's requirements.txt.

[tgbugs]

Ok. I think I have a solution. If in install_requires you list neurdflib after prov then neurdflib will be installed second, and its version of site-packages/rdflib will replace the version from 4.2.2.

EDIT: The above is false, I forgot to run unset PYTHONPATH so I was pulling in my repo. Sigh.

[satra]

@tgbugs - i did test in a container and it seemed to work - just pushed the changes here to test on circle.

[tgbugs]

Huh, ok. Maybe it did work, my concern would be that the order may not be deterministic, so sometimes neurdflib would install first, and other times rdflib would.

[effigies]

You could try making rdflib a dependency of neurdflib. That should impose an installation order.

[effigies]

Will we need to make a neurdflib-feedstock, or can conda packages have pip dependencies? If so, it'd be good to get that going ASAP so we can test that the installation order in conda is deterministic.

[tgbugs]

I have never used conda so I don't know whether it will pull in packages from pypi that are listed in requirements.txt or install_requires, if it does pull them in, then I would imagine that the feedstock is not required. @satra do you know more about this?

[satra]

this is what the recipe looks like:

https://github.com/conda-forge/nipype-feedstock/blob/master/recipe/meta.yaml#L18

so the only place rdflib comes in is the run section, i'll have to check if they have changed what it can be. in the past the dependency structure had to be a version on the forge.

[effigies]

I see nothing in the docs that indicates conda can pull in from pip during the installation.

So I think we would need a neurdflib-feedstock if we want to allow conda installation. Otherwise, we could make it work for pip, and tell people to pip install neurdflib if they want to use provenance.

[satra]

@tgbugs - for conda, we will need a feedstock for neurdflib. you could probably just copy the recipe from the rdflib-feedstock.

@effigies - we could put a check in config.enable_provenance() to check that the correct version of rdflib is being used. (i can add it in)

[tgbugs]

Ok will take a look.

EDIT: can use this as a base https://github.com/conda-forge/rdflib-feedstock

[effigies]

At this point I'm a little worried about trying to get the conda packages aligned before Monday. I think if we update the dependency, and the conda package is not there, we're just asking for things to break, such as nipypecli (see #2680).

We could push the release off by a day or two, if that would help. Or we could plan on a 1.1.4 release that is just 1.1.3 + #2701, and we can kick off that process as soon as neurdflib exists on conda-forge.

[satra]

@effigies - i agree that this is not essential for this release. it's good to know that we have some hackish solutions.

we can punt this to 1.1.4, together with an actual alignment of provenance to current NIDM-W.

and perhaps we will get a response on our comment in the rdflib PR.

[effigies]

Sounds good.

[tgbugs]

@satra did you ever encounter an error like this while trying to build rdflib for conda? https://ci.appveyor.com/project/conda-forge/staged-recipes/build/1.0.28125

I have an inkling that it is because of the colons in the filename, since it works when I test using 7zip on windows which replaces the colons with underscores.

[satra]

@tgbugs - don't know why that error would happen now relative to all the rdflib builds - i would ping the conda maintainers for help.

[tgbugs]

The recipe checks have passed so now we just wait for the feedstock repo to appear I think.

[effigies]

@satra What's left to do on this one?

[satra]

feedstock is available now. so i'm fine with this upgrade path. anything that you can think of that we should improve.

[effigies]

I'm feeling lucky.