-
Notifications
You must be signed in to change notification settings - Fork 10
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
Rewriting ontodoc based on domain-battery #742
Conversation
If not the later tests fail (since a global variable is changed)
After simplifying paths in test_save_emmo_domain_ontology() in test_save.py to make it possible to debug, the test works again :-)
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #742 +/- ##
==========================================
+ Coverage 72.11% 73.22% +1.11%
==========================================
Files 17 18 +1
Lines 3486 3683 +197
==========================================
+ Hits 2514 2697 +183
- Misses 972 986 +14 ☔ View full report in Codecov by Sentry. |
tests/test_ontodoc_rst.py
Outdated
from ontopy.ontodoc_rst import OntologyDocumentation | ||
import owlready2 | ||
|
||
emmo = get_ontology("https://w3id.org/emmo/1.0.0-rc1").load() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am not so happy about tests that dowload EMMO. We already have many of those and they make the tests very slow.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Replaced it with the local animal ontology.
str(test_file), | ||
str(outdir / "mammal.rst"), | ||
] | ||
) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does this test what is actually created?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added simple tests for generated content...
tests/test_save.py
Outdated
assert not {"isq.rdfxml", "catalog-v001.xml"}.difference( | ||
os.listdir(outputdir / "emmo.info" / "emmo" / "disciplines") | ||
) == {"isq.rdfxml", "catalog-v001.xml"} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Because onto.save() stores other files in addition to isq.rdfxml and the catalogue file. Explicitly checking for all created files will fail when new modules are added to EMMO disciplines.
tests/test_save.py
Outdated
assert set( | ||
os.listdir(outputdir / "emmo.info" / "emmo" / "disciplines") | ||
) == {"isq.rdfxml", "catalog-v001.xml"} | ||
assert not {"isq.rdfxml", "catalog-v001.xml"}.difference( | ||
os.listdir(outputdir2 / "emmo.info" / "emmo" / "disciplines") | ||
) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why this? It is more difficult to read.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree that it is more difficult to read. But without this, the test is failing since onto.save() creates other files in addition to isq.rdfxml and the catalogue file.
We can either explicitly checking for all created files, but that will fail when new modules are added to EMMO disciplines.
Would it be more readable to rewrite the test as
created_files = set(os.listdir(outputdir2 / "emmo.info" / "emmo" / "disciplines"))
for fname in ("chameo.rdfxml", "catalog-v001.xml"):
assert fname in created_files
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes, I prefer this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Approved, but I prefer the change in one of the tests as you propose in the comment
Description
Rewriting and simplifying the ontodoc tool based on domain-battery. For now the python module is called ontodoc_rst.py.
Structure the generated document as follows:
For each module, document:
Tasks
index.rst
(just a template that the user can improve).rst
output. It doesn't run sphinx automatically, though...The last points can maybe be done in a separate PR.
Type of change
Checklist
This checklist can be used as a help for the reviewer.
Comments