-
Notifications
You must be signed in to change notification settings - Fork 23
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
Feature 1419 competency question coverage report #1420
Conversation
I am still to implement the badge. There is a very important caveat here. The tests can't run with the owl file because for some reason the translation is not correctly pasing some labels. I have to look at it from a linux computer because my work pc is windows and I have to do a lot of things from imagination. Related issue: #1424 |
Im going to be working on implementing the badge in a personal fork because I have to play around with the secrets and such. When I have it figure out I will come to you and tell you exactly the steps to be followed |
Take a look at the README of this fork branch, that is how the badge would look like: https://github.com/areleu/ontology/tree/feature-1419-competency-question-coverage-report To implement it here I would need to support from someone with owner rights to set up the secrets correctly etc. |
Now the tests fail if the competency questions are not fulfiled. Its missing a way of ruling out CQs. But In general I would encourage against that. Something that I like from this implementation is that you get feedback in the CI itself on which tests failed. |
Looking at this PR, where did we decide to rename all the files? This PR goes far beyond what was discussed in the issue. |
It was part of the proposed structure:
But I can easily revert back to the old one and it should still work, what I would definitively do if we dont revert is make sure that the numbering is consistent with the old numbering, for traceability. |
…uplicate questions
I made sure the numbers now are consistent with the old ones. Also restored deleted questions, even the duplicates. |
How does This PR changes too much things at once, this makes it very hard to review. |
|
||
# This competency question gets deprecated with this pull request: https://github.com/OpenEnergyPlatform/ontology/pull/1409 | ||
# This means it should be false |
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.
To me, the answer this competency question is neither false not right, but the question is undecidable, because one needs additional information about the biofuel to decide this question. Same with Q29.
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.
Then I will handle this differently, I will add a folder where all the deprecated questions are to me moved, without changing their conditions at all. I will also add the option to run the deprecated questions by adding the "--deprecated" flag to the pytest call.
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 think that is a good solution.
This file can be deleted, test_competency_questions.py replaces it |
Are any of the changes of this PR documented? I do not see any entries in CHANGELOG.md. In the past we documented there also changes to the competency questions and scripts, e.g. Line 27 in cd70e76
Line 117 in cd70e76
|
How is the status and this PR and should it be considered for the release 1.16.0 next Tuesday? @areleu |
I can rebase it and it will be ready |
Summary of the discussion
This is the branch I will work on for a coverage report. It builds on #1319
Type of change (CHANGELOG.md)
Added
Updated
Removed
Workflow checklist
Automation
Closes #1419
PR-Assignee
term tracker item
Reviewer