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

Python3 version #34

Merged
merged 6 commits into from
Nov 1, 2020
Merged

Python3 version #34

merged 6 commits into from
Nov 1, 2020

Conversation

iherman
Copy link
Contributor

@iherman iherman commented Sep 7, 2020

Finalized a Python 3 version

@iherman iherman requested a review from nicholascar September 7, 2020 13:59
@iherman iherman mentioned this pull request Sep 9, 2020
Copy link
Member

@nicholascar nicholascar left a comment

Choose a reason for hiding this comment

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

In the changes to file Doc-pyRdfa/pyRdfa.host-module.html, pyRdfa/__init__.py & pyRdfa/host/__init__.py, you've stated that dependencies are:

<a href="http://rdflib.net" target="_top">RDFLib package</a> version 5 or higher

and

Python version 2.7 or 3.8 or higher

These are both incorrect: For the first point, I'm not sure this module now works with RDFlib 5.0.0 and for the second, RDFlib 5.0.0 works with Python 2.7 & 3.4+.

Otherwise changes seem fine, if somewhat out-of-date since these changes were mad in January? i.e. before RDFlib 5.0.0.

Any reason to note 3.8+ as you've done? Of course you can have whatever restrictions you like in this plugin but best would to be in sync with RDFlib core.

RDFlib 6.0.0 will be Python 3.6+.

@iherman
Copy link
Contributor Author

iherman commented Sep 9, 2020

@nicholascar thanks. I realized that, actually, there was a discrepancy in the documentation, so I had to regenerate it...

That being said:

I'm not sure this module now works with RDFlib 5.0.0 and for the second, RDFlib 5.0.0 works with Python 2.7 & 3.4+.

I am not sure how the dates got tangled, but the changes were actually made in April. I do not remember exactly when 5.0.0 came out, but that is certainly the version I run on my machine and I tested with. I did not test this with earlier versions of RDFLib, hence that dependency.

As for the 3.4 vs. 3.8: I must admit I do not remember the details. What I do know (because I was pretty upset about it) is that there was some specific, and backward-incompatible change between 3.8 and a previous version, which created some problems with my code. I know I had to fight with this sometimes at the beginning of the year; however, subsequent private events (some health issues, COVID lockdown in France, etc...) wiped out my memory. But I do know that I needed to upgrade my local Python3 version to get things running. So yes, I am afraid Python3.8 is necessary for this module:-(

(Sorry to be a bit vague...)

@FlorianLudwig
Copy link

@iherman @nicholascar I tried the lib with python3.5 and python3.6 by running scripts/localRDFa.py on a small html file and had no issues.

With even python 3.5 having reached end of life, I would not spend any time on python 3.4.

@nicholascar
Copy link
Member

I’ll merge this shortly and review state of the module in general.

We are preparing to incorporate JSON-LD and a few other bits and pieces into RDFlib core, so if dependencies can be aligned, this can go in too and thus both increase RDFLib capabilities and reduce maintenance burden. We will have to test this integration all properly of course, but that’s do-able.

@iherman
Copy link
Contributor Author

iherman commented Oct 31, 2020

B.t.w., @nicholascar, I think I have taken care of your issues above, maybe you want to update your review...

@nicholascar nicholascar merged commit 84ae7b1 into master Nov 1, 2020
@nicholascar nicholascar deleted the python3-version branch November 1, 2020 04:10
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.

3 participants