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

Migrate to Python3 #70

Open
wants to merge 25 commits into
base: main
Choose a base branch
from
Open

Migrate to Python3 #70

wants to merge 25 commits into from

Conversation

papoteur-mga
Copy link

For the migration, I started from the last state of the main branch, then applied the patches from the python3 branch.
Thus the last commits from the branch peds are not used,

Copy link
Member

@amotl amotl left a comment

Choose a reason for hiding this comment

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

Thank you SO MUCH for tackling this!

I've just added a few suggestions while quickly having a look. I look forward to merge your patch soon, but please give me some time to properly review it, eventually add some fixes about shortcomings you've outlined at GH-68, and address further details.

docs/conf.py Outdated Show resolved Hide resolved
setup.py Outdated
Comment on lines 218 to 222
dependency_links=[
'https://github.com/ip-tools/mongodb_gridfs_beaker/archive/0.6.0dev1.tar.gz#egg=mongodb_gridfs_beaker',
'https://github.com/ip-tools/mechanize/archive/v0.4.3dev2.tar.gz#egg=mechanize-0.4.3dev2',
#'https://github.com/dagwieers/unoconv/archive/master.tar.gz#egg=unoconv-0.8.2',
],
Copy link
Member

@amotl amotl Mar 28, 2023

Choose a reason for hiding this comment

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

Note to self: In order to be able to run a general availability release, and publish the package to PyPI, we will need to resolve those dependencies beforehand.

Copy link
Author

Choose a reason for hiding this comment

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

Can it be solved by including submodules in the source path?

Copy link
Member

Choose a reason for hiding this comment

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

We could vendorize the dependencies, but we still will need to port them over to Python 3. I think there is not so much work to do. On mongodb_gridfs_beaker, I made a start at 1, which is probably already pulled in here. On mechanize, a minor patch was required, but it's already upstream, see 2.

Footnotes

  1. https://github.com/loucash/mongodb_gridfs_beaker/pull/1

  2. https://github.com/python-mechanize/mechanize/pull/43

@amotl

This comment was marked as resolved.

@amotl amotl mentioned this pull request Mar 29, 2023
@amotl amotl changed the title Port to Python3 Migrate to Python3 Mar 29, 2023
@codecov-commenter
Copy link

codecov-commenter commented Mar 29, 2023

Codecov Report

Patch coverage: 33.25% and project coverage change: -7.47 ⚠️

Comparison is base (0933634) 51.75% compared to head (c7af549) 44.28%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main      #70      +/-   ##
==========================================
- Coverage   51.75%   44.28%   -7.47%     
==========================================
  Files         100      100              
  Lines        7770     7878     +108     
==========================================
- Hits         4021     3489     -532     
- Misses       3749     4389     +640     
Flag Coverage Δ
unittests 44.28% <33.25%> (-7.47%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
patzilla/access/epo/espacenet/pyramid.py 32.25% <ø> (ø)
patzilla/access/epo/ops/api.py 11.44% <0.00%> (-58.32%) ⬇️
patzilla/access/epo/publicationserver/client.py 37.03% <0.00%> (+2.42%) ⬆️
patzilla/access/generic/exceptions.py 42.85% <0.00%> (-13.25%) ⬇️
patzilla/access/ificlaims/api.py 16.35% <0.00%> (+0.52%) ⬆️
patzilla/access/office.py 15.21% <0.00%> (+1.88%) ⬆️
patzilla/access/uspto/pdf.py 30.76% <0.00%> (+0.76%) ⬆️
patzilla/navigator/services/__init__.py 46.66% <0.00%> (ø)
patzilla/navigator/services/admin.py 24.19% <0.00%> (+2.52%) ⬆️
patzilla/navigator/services/dpma.py 27.27% <0.00%> (+4.10%) ⬆️
... and 57 more

... and 11 files with indirect coverage changes

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

Before, the system would happily use empty credentials induced by CI
when running from foreign repositories, where the GitHub secrets
attached to the repository are not active, but the GHA workflow
configuration still sets the environment variables - to empty values.
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.

None yet

4 participants