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.

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

Update requirements #216

Merged
merged 11 commits into from
Sep 22, 2021
Merged

Update requirements #216

merged 11 commits into from
Sep 22, 2021

Conversation

CasperWA
Copy link
Contributor

@CasperWA CasperWA commented Sep 21, 2021

This does not close the #206 issue anymore. See the comment below.
Closes #206.
This now closes #206, because it has been decided to keep support for rdflib<6.0.0 in order to support system-wide installation and use of EMMOntoPy. With the inclusion of the warnings here, we also let users know that if they want to properly output turtle files (for certain edge-cases) they should ensure they can install the latest rdflib version.

Set version lofts for Python dependencies.

Note, this is a remake of #212, but using a branch from this repository instead of a fork.

Since 6.0.0 is the immediate release after 5.0.0, this makes sense. And
it's nicer to have `>=` dependencies than `>`.

Note, there is an issue with 5.0.0 that produces wrong turtle files.
An error will be thrown if this is not satisfied.
This is also true for the `ontoconvert` CLI tool.
@CasperWA CasperWA force-pushed the cwa/close-206-required-rdf-5 branch from 6d28640 to 4de4f97 Compare September 21, 2021 09:14
@CasperWA
Copy link
Contributor Author

@francescalb This has been remade from my forked branch, so it should be easier to test locally now without having to add my fork as a remote :)

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.

Should be included in ontology.save as well. Now saving a new ontology in .ttl format with rdflib 4.2.2 does not return an error.

@francescalb
Copy link
Collaborator

I am a little unsure if it is not best to give a Warning rather than an error. It depends on the scientific notation is commonly used or not.

@CasperWA
Copy link
Contributor Author

I am a little unsure if it is not best to give a Warning rather than an error. It depends on the scientific notation is commonly used or not.

Fine by me.
Especially after looking at the comment in the README. It indeed seems to be for specific cases.

@francescalb
Copy link
Collaborator

The whole warning when saving is not very pretty, but maybe it should be like that?

from ontopy import get_ontology
o = get_ontology('emmo').load()
o.save('ontology.ttl')
/full/path/to/EMMO-python/ontopy/ontology.py:506: IncompatibleVersion: To correctly convert to Turtle format, rdflib must be version 6.0.0 or greater, however, the detected rdflib version used by your Python interpreter is '4.2.2'. For more information see the 'Known issues' section of the README.
warnings.warn(

@CasperWA
Copy link
Contributor Author

The whole warning when saving is not very pretty, but maybe it should be like that?

This is the standard Python warning message formatting. The formatting can be changed, however, this should be subject of a separate PR I think, otherwise this particular PR will be bogged down with warning formatting discussions and not with a discussion regarding the requirements, as it should.

I suggest you create an issue from your comment and maybe outline how you would like it to be formatted. Or just at least create the issue, then we can discuss it later :)

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.

Now works very nicely.

@CasperWA
Copy link
Contributor Author

As a note, I've made the warning "pretty" for the CLI tool :)

@francescalb
Copy link
Collaborator

As a note, I've made the warning "pretty" for the CLI tool :)

I noticed :)

@CasperWA CasperWA merged commit 0126c03 into master Sep 22, 2021
@CasperWA CasperWA deleted the cwa/close-206-required-rdf-5 branch September 22, 2021 09:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Require rdflib>5.0.0?
2 participants