Skip to content

Kmp 2to3 fixes poetry (C4-1106) #3

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

Merged
merged 24 commits into from
Oct 5, 2023
Merged

Conversation

netsettler
Copy link

This adds poetry, PEP8, and some fixes to the tests and the repo.

Copy link
Member

@willronchetti willronchetti left a comment

Choose a reason for hiding this comment

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

Looks good should be tested in CGAP though

# This workflow contains a single job called "build"
build:
# The type of runner that the job will run on
runs-on: ubuntu-20.04
Copy link
Member

Choose a reason for hiding this comment

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

22.04?

Copy link
Author

Choose a reason for hiding this comment

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

Thanks. Fixed.

name: Python ${{ matrix.python_version }} Tests

# The type of runner that the job will run on
runs-on: ubuntu-20.04
Copy link
Member

Choose a reason for hiding this comment

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

22.04?

Copy link
Author

Choose a reason for hiding this comment

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

Fixed, too.

runs-on: ubuntu-20.04
strategy:
matrix:
python_version: ['3.7', '3.8', '3.9', '3.10', '3.11']
Copy link
Member

Choose a reason for hiding this comment

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

No need for 3.7 I'd say

setup.py Outdated
@@ -1,26 +1,31 @@
import glob
Copy link
Member

Choose a reason for hiding this comment

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

This file can be deleted entirely no?

Copy link
Author

Choose a reason for hiding this comment

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

Seemingly. It took some adjusting but seems to be working now.

version.py Outdated
@@ -0,0 +1 @@
vcf/__init__.py
Copy link
Member

Choose a reason for hiding this comment

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

Can be deleted no?

Copy link
Author

Choose a reason for hiding this comment

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

Had to reimplement how it __init__.py gets the version, but yeah, I don't think version.py is doing anything.

Copy link
Author

Choose a reason for hiding this comment

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

In my revised version, it gets the VERSION from pyproject.toml (indirectly through importlib.metadata) whereas previously __init__.py was the source of authority on version.

@netsettler netsettler requested a review from B3rse October 4, 2023 18:57
@netsettler netsettler changed the title Kmp 2to3 fixes poetry Kmp 2to3 fixes poetry (C4-1106) Oct 4, 2023
@netsettler
Copy link
Author

This tested cleanly in cgap-portal testing for Python 3.8 on my local machine.

poetry run python -m pytest -vv -m static && make lint
===== 3 passed, 2175 deselected, 238 warnings in 8.75s =====
poetry run python -m pytest -xvv -r w --durations=25 --timeout=600 -m "not manual and not sloppy and not static and not broken and not performance and not integratedx and not indexing"
===== 1087 passed, 15 skipped, 1076 deselected, 944 warnings in 751.99s (0:12:31) =====
poetry run python -m pytest -xvv -r w --durations=25 --timeout=200 -m "not manual and not sloppy and not static and not broken and not performance and not integratedx and indexing and es"
===== 20 passed, 2158 deselected, 647 warnings in 177.56s (0:02:57) =====
poetry run python -m pytest -xvv -r w --durations=25 --timeout=200 -m "not manual and not sloppy and not static and not broken and not performance and not integratedx and indexing and not es"
===== 616 passed, 72 skipped, 1490 deselected, 1609 warnings in 121.91s (0:02:01) =====
commit 0467c8be1fd8b8a86ada9d6d110e436f424ab039 (HEAD -> upgrade-python-20230925, origin/upgrade-python-20230925)
Wed Oct  4 19:00:34 EDT 2023

Also in Python 3.11 on GA.

@netsettler
Copy link
Author

OK, I've moved the label to 3.0.0 since, by agreement with @willronchetti on Slack, passing tests in cgap-portal is a good test. No need for test deploy, he also agreed. (And, as a contingency in case of later problems, if anyone needs to revert, they can always use ^2.0.0 and they won't end up in the 3.0.0 series.) I think this is ready to merge.

@netsettler netsettler merged commit 89926e2 into master Oct 5, 2023
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.

2 participants