Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Fix ontoconvert rdflib import #307

Merged
merged 5 commits into from
Dec 2, 2021
Merged

Fix ontoconvert rdflib import #307

merged 5 commits into from
Dec 2, 2021

Conversation

CasperWA
Copy link
Contributor

Description:

Fixes #306

Graph should be imported from rdflib.graph in the current version of rdflib.

Added simple tests for ensuring the main() functions for each tool runs correctly.

Type of change:

  • Bug fix.
  • New feature.
  • Documentation update.

Checklist:

This checklist can be used as a help for the reviewer.

  • Is the code easy to read and understand?
  • Are comments for humans to read, not computers to disregard?
  • Does a new feature has an accompanying new test (in the CI or unit testing schemes)?
  • Has the documentation been updated as necessary?
  • Does this close the issue?
  • Is the change limited to the issue?
  • Are errors handled for all outcomes?
  • Does the new feature provide new restrictions on dependencies, and if so is this documented?

Comments:

jesper-friis
jesper-friis reviewed Nov 30, 2021
View reviewed changes
tests/tools/conftest.py
@@ -0,0 +1,34 @@
from typing import TYPE_CHECKING
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not smart enough to see what actually are testing here. Maybe you can add a comment

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is for type annotations only. It has nothing to do with testing, but rather to support IDEs as well as set the package up for future testing with MyPy (see #223).

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the explanation. I think it would be good to have as a module docstring.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will be littered all over the code those in the future - Maybe I'll add it to the developer's documentation instead? Or do you want a comment in all module doc-strings where it'll be?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added a new documentation page about testing and tools for developers :)

jesper-friis
jesper-friis reviewed Nov 30, 2021
View reviewed changes
Copy link
Collaborator

@jesper-friis jesper-friis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Adds simple tests of all tools. Nice

francescalb
francescalb approved these changes Dec 1, 2021
View reviewed changes
Copy link
Collaborator

@francescalb francescalb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks nice, good to have tests for the tools as well!

@CasperWA CasperWA mentioned this pull request Dec 2, 2021
Closed
11 tasks
@CasperWA CasperWA force-pushed the cwa/fix-306-ontodoc-rdflib-import branch from f908df2 to 5cb20d2 Compare December 2, 2021 17:31
@CasperWA CasperWA enabled auto-merge December 2, 2021 17:32
@CasperWA CasperWA disabled auto-merge December 2, 2021 17:32
@CasperWA CasperWA enabled auto-merge December 2, 2021 17:32
@CasperWA CasperWA changed the title Fix ontodoc rdflib import Fix ontoconvert rdflib import Dec 2, 2021
@CasperWA CasperWA merged commit 4dc1e97 into master Dec 2, 2021
@CasperWA CasperWA deleted the cwa/fix-306-ontodoc-rdflib-import branch December 2, 2021 17:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@francescalb francescalb francescalb approved these changes

@jesper-friis jesper-friis jesper-friis approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Ontodoc failing due to wrong rdflib import
3 participants
@CasperWA @jesper-friis @francescalb