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

Python 3 compatibility #86

Open
wants to merge 12 commits into
base: master
Choose a base branch
from
Open

Conversation

loleg
Copy link
Member

@loleg loleg commented Oct 31, 2023

Still testing, this bumps the dependencies and uses the unpublished git master of https://github.com/CLARIAH/iribaker to make the plugin work with Python 3.

Additionally I ran 2to3 on all .py files and tests to fix syntax, and resolved some linter issues.

requirements.txt Outdated
iribaker==0.1.2
python-dateutil==2.8.1
rdflib==5.0.0
iribaker==git+https://github.com/CLARIAH/iribaker.git#egg=iribaker
Copy link
Member Author

Choose a reason for hiding this comment

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

I raised an issue at https://github.com/CLARIAH/iribaker/issues about this ...

Choose a reason for hiding this comment

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

@loleg
Copy link
Member Author

loleg commented Nov 22, 2023

The CKAN test stumbles on the Python compatibility issue. I'm not sure why it's not loading the same version as the linter, which is passing. Excerpt of the error:

    import iribaker
  File "/srv/app/src/iribaker/iribaker/__init__.py", line 1, in <module>
    import urllib.parse
ImportError: No module named parse

@bellisk
Copy link
Contributor

bellisk commented Nov 22, 2023

@loleg Both the tests and the linter are being run on ubuntu-latest, but the tests themselves are being run inside a container inside the runner. The container is built on the openknowledge/ckan-dev image tagged for CKAN v2.8, so it's still running Python 2.

Relevant part of test.yml:

    strategy:
      matrix:
        ckan-version: [2.8]
      fail-fast: false
    name: CKAN ${{ matrix.ckan-version }}
    runs-on: ubuntu-latest
    container:
      image: openknowledge/ckan-dev:${{ matrix.ckan-version }}

opendata.swiss is unfortunately running CKAN 2.8 and Python 2, so we need this extension to retain compatibility with those. :( I would suggest adding tests for more versions:

      matrix:
        ckan-version: [2.8, 2.9, 2.9-py2, "2.10"]

Then at least you'll be able to see if the tests are passing with Python 3.

@bellisk
Copy link
Contributor

bellisk commented Nov 22, 2023

@loleg I forgot to say, thank you for the work on this! 🎉

@loleg
Copy link
Member Author

loleg commented Dec 6, 2023

On 2.8 it's failing due to iribaker which is not backwards-compatible. The errors on 2.9 don't make any sense. Failing on 2.10 due to this:

Collecting rdflib-jsonld==0.5.0 (from -r https://raw.githubusercontent.com/ckan/ckanext-dcat/0c26bed5b7a3a7fca8e7b78e338aace096e0ebf6/requirements-py2.txt (line 2))
  Downloading rdflib-jsonld-0.5.0.tar.gz (55 kB)
     ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ 55.9/55.9 kB 135.6 MB/s eta 0:00:00
  Preparing metadata (setup.py): started
  Preparing metadata (setup.py): finished with status 'error'
  error: subprocess-exited-with-error
  
  × python setup.py egg_info did not run successfully.
  │ exit code: 1
  ╰─> [1 lines of output]
      error in rdflib-jsonld setup command: use_2to3 is invalid.
      [end of output]

You know that song..."Who let the dogs out? Woof! Woof!" 🙈

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