incf-nidash / nidm-specs

Neuroimaging Data Model (NIDM): describing neuroimaging data and provenance
nidm.nidash.org
Other
33 stars 30 forks source link

Assignment of NIDM terms to prov categories in owl_reader.py #453

Closed khelm closed 6 years ago

khelm commented 6 years ago

@cmaumet : If I understand the code correctly, I see that owl_reader.py tries to figure out which of the prov categories (activity, entity, agent) a given term should be associated with. I see the "recursive=3" assignment - does that mean that it goes up three graph nodes to see if it hits one of the prov categories? This is in the module get_class_name_by_prov_type.

This module returns a warning for most, maybe all (I haven't gone through each one to find out) of the nidm namespace terms. All of the nidm namespace terms should return a prov category- could you have your code search through the graph until it hits the prov category?

khelm commented 6 years ago

I tried increasing the recursive level in my version of owl_reader.py and the warnings still occur for all of the terms when running create_expe_specifications.py with the terms included in that code. So clearly there is some other issue that is giving the warnings. Also, the specs page for nidm-experiment does not update after merging my updates.

cmaumet commented 6 years ago

Hi @khelm. I just had a look at the NIDM-experiment ontology file and there are a number of (SIO) terms that are not defined as child of a PROV term which explains why we are getting the warnings:

screen shot 2018-02-27 at 09 35 43

As NIDM is based on PROV, the script that generates the HTML spec assumes that each new term is a descendant of a PROV term.

Do you think it would be possible to have those SIO terms under one of the PROV terms (Entity/Activity/Agent/...)?

khelm commented 6 years ago

Hi @cmaumet - this is not what my current file gives in protege. It's unclear to me why yours should look like that - is there a missing or old imports file?

Also the error I get about the terms not being in PROV are not for the SIO terms. These are for all new terms, e.g., .../python2.7/site-packages/nidmresults/owl/owl_reader.py:234: UserWarning: No PROV type for class: nidm:SingleUnitRecording self.graph.qname(class_name))

khelm commented 6 years ago

2018-02-27-114534_1366x768_scrot

cmaumet commented 6 years ago

Hi @khelm! Hum... I've just double checked and still get the same output... Can you run git status and git log -2 and paste in what they return to check why we are not in sync? Here are my outputs:

$ git status 
On branch master
Your branch is up to date with 'upstream/master'.

nothing to commit, working tree clean
$ git log -2
commit a35b64d357d9e206126e6a2fbc91d0f2b916c997 (HEAD -> master, upstream/master, upstream/HEAD)
Merge: bee9202d c5b4da40
Author: Camille Maumet <cmaumet@users.noreply.github.com>
Date:   Tue Feb 27 11:24:53 2018 +0100

    Merge pull request #454 from incf-nidash/cmaumet-patch-1

    Update link to build status in README

commit c5b4da402ffa4a8d32906d64e23a0f0deb061315 (cmaumet-patch-1)
Author: cmaumet <camille.maumet@inria.fr>
Date:   Tue Feb 27 11:15:46 2018 +0100

    python scripts/refresh.py
khelm commented 6 years ago

I get:

Switched to branch 'master'
Your branch is up-to-date with 'upstream/master'.
$ git log -2
commit bee9202d8b35790cc563b53e25be0c61bd0a34b2
Merge: eb8859a 89df9a3
Author: Karl Helmer <helmer@nmr.mgh.harvard.edu>
Date:   Wed Feb 21 10:23:27 2018 -0500

    Merge pull request #452 from khelm/nidm-e-specs

    update create_expe_specifications.py with current terms

commit 89df9a345e8ab7694dc991821cee9b64d268f8d5
Author: Karl Helmer <helmer@nmr.mgh.harvard.edu>
Date:   Thu Feb 15 13:52:11 2018 -0500

    remove fMRI design type terms from create_expe_specification.py and add add 
cmaumet commented 6 years ago

Thanks @khelm, this confirms that we are not in sync :)

Can I double check that the first log above is the output of git status? I don't understand why the log starts with "Switched to branch" while this command should only display current status.

We should probably update your copy of upstream as it looks like it is outdated compared to what we have online (last merged PRs are missing) but before it is worth making sure we understand in which state your local repo is (with git status).

Can you also send out which version of git you are using? For me:

$ git --version
git version 2.15.0
khelm commented 6 years ago

I believe it says that because I was on a different branch and had just switched to master before doing git status. I wonder if this has to do with the name change to nidm-specs? I just updated my upstream definition and merged my local master with upstream and it says that everything is up-to-date, but then, it said that before. I'm using git 1.9.1 which is evidently the latest on Linux.

khelm commented 6 years ago

Also, the master on github has my latest changes (incorporating the fMRI Design Types as NIDM ID's) and this doesn't have the issue with the SIO terms that you see.

cmaumet commented 6 years ago

(We've discussed this with @khelm and identified an issue with the import files, now addressed in #458).