INCATools / ontology-development-kit

Bootstrap an OBO Library ontology
http://incatools.github.io/ontology-development-kit/
BSD 3-Clause "New" or "Revised" License
223 stars 54 forks source link

Refreshing of mirrors should not be blocked by IMP=false #946

Closed gouttegd closed 9 months ago

gouttegd commented 11 months ago

Currently, the generated rule that refreshes a mirror only does something when both MIR and IMP are true:

mirror-cl:
    if [ $(MIR) = true ] && [ $(IMP) = true ]; then ... ; fi

This makes it impossible to have a custom workflow that depends on a mirror and that could be run under IMP=false. Such a workflow would have to be run under IMP=true in order to be sure the required mirror has indeed been downloaded and is available locally.

Requiring both MIR and IMP to be true for a mirror to be refreshed only makes sense if we assume that the only purpose of a mirror is to be used to create an import module. That may be true in many (possibly most) cases, but there are definitely situations where one might want to use a locally mirrored foreign ontology for other things in addition to creating an import module. Uberon for example is re-using the locally mirrored CL and ZFA ontologies as part of the bridge generation pipeline, which has nothing to do with the imports pipeline.

The only benefit of the current approach that I can see is that it allows user to just have to specify IMP=false for completely bypassing both the imports pipeline and the mirroring pipeline. But the price for that is what is to me an inconsistent behaviour: if I run a workflow with MIR=true IMP=false, I would expect the mirrors to be refreshed (MIR=true) and the import modules not being re-generated (IMP=false).

I believe the rules that refresh the mirror should strictly depend on MIR only.

matentzn commented 11 months ago

Requiring both MIR and IMP to be true for a mirror to be refreshed only makes sense if we assume that the only purpose of a mirror is to be used to create an import module.

This is indeed the historical reason. I agree with your proposal! Lets have mirrors only depend on MIR.