-
Notifications
You must be signed in to change notification settings - Fork 28
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
Upgrade Python 3.11 #396
Upgrade Python 3.11 #396
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
With Tibanna, you need to change the lambda declaration as well ie: https://github.com/4dn-dcic/tibanna/blob/master/tibanna/lambdas/check_task_awsem.py#L10
Will approve once done
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@willronchetti, there's no testing in GA for this? Does foursight pick up the slack?
My other comments are not blocking.
.github/workflows/main-publish.yml
Outdated
# This workflow contains a single job called "build" | ||
build: | ||
# The type of runner that the job will run on | ||
runs-on: ubuntu-20.04 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can this work with 22.04?
@@ -5,6 +5,9 @@ clean: | |||
lint: | |||
flake8 tibanna | |||
|
|||
build: | |||
poetry install |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd add make configure
here. You'll need to make a target.
And install
looks redundant with build
. Rather than eliminate it, to stay compatible, I suggest it should identify itself a synonym and then be implemented by make build
.
pyproject.toml
Outdated
python-lambda-4dn = "0.12.3" | ||
boto3 = "^1.9.0" | ||
botocore = "^1.12.1" | ||
boto3 = "^1.28.51" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe add boto3-stubs and botocore-stubs in dev-dependencies.
There are unit tests on GA for this, yes |
Ah, I see. @dmichaels-harvard it looks like main.yml workflow got overlooked, unless it's too expensive to run more than one (@willronchetti?) |
@@ -22,6 +22,7 @@ Version updates | |||
**Sep 15, 2023** The latest version is now 4.0.0_. | |||
- Support for Python 3.7 has been dropped | |||
- Added support for Python 3.9 and 3.10 | |||
- Added support for Python 3.11 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sep 21, 2023 The latest version is now 5.0.0_.
- Added support for Python 3.11
@@ -43,7 +43,7 @@ API().run_workflow(input_json='myrun.json') | |||
--- | |||
Note: Starting `0.8.2`, Tibanna supports local CWL/WDL files as well as shell commands and Snakemake workflows. | |||
|
|||
Note 2: As of Tibanna version `2.0.0`, Python 3.6 is no longer supported. Please switch to Python 3.8! Python 3.7 is also supported as a fallback, but please prefer 3.8 if you can. | |||
Note 2: As of Tibanna version `2.0.0`, Python 3.7 (and lower) is no longer supported. Please switch to Python 3.11! Python 3.8 is also supported as a fallback, but please prefer 3.11 if you can. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tibanna 3 supports Python 3.7. Better include a new block:
Note 3: As of Tibanna version 5.0.0
, Python 3.7 (and lower) is no longer supported. Please switch to Python 3.11!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good. I deployed this version to cgap-wolf and ran a test workflow successfully.
- Do the local tests run successfully with Python 3.11? You need 4DN account credentials sourced to run these. I would have run that myself, but somehow I am having trouble to set up a 3.11 environment.
- I left some minor comments above
- Build error must be fixed
- Needs version bump
- Let me know when you release the Pypi package. I need to build and upload the corresponding Docker image at the same time.
No description provided.