Skip to content

change: Update required python version#219

Merged
fege merged 11 commits intoopendatahub-io:mainfrom
fege:python_required
Apr 11, 2025
Merged

change: Update required python version#219
fege merged 11 commits intoopendatahub-io:mainfrom
fege:python_required

Conversation

@fege
Copy link
Copy Markdown
Contributor

@fege fege commented Apr 4, 2025

Description

Update required python version, when using 3.9 version there are some import that are not available for example

    from typing import Self
E   ImportError: cannot import name 'Self' from 'typing' 

This was added only in 3.11 typing.Self
Also | for Union was added in 3.10 typing.Union

How Has This Been Tested?

Merge criteria:

  • The commits are squashed in a cohesive manner and have meaningful messages.
  • Testing instructions have been added in the PR body (for PRs involving changes that are not immediately obvious).
  • The developer has manually tested the changes and verified that the changes work

@fege fege requested a review from a team as a code owner April 4, 2025 09:47
@github-actions
Copy link
Copy Markdown

github-actions bot commented Apr 4, 2025

The following are automatically added/executed:

  • PR size label.
  • Run pre-commit
  • Run tox
  • Add PR author as the PR assignee

Available user actions:

  • To mark a PR as WIP, add /wip in a comment. To remove it from the PR comment /wip cancel to the PR.
  • To block merging of a PR, add /hold in a comment. To un-block merging of PR comment /hold cancel.
  • To mark a PR as approved, add /lgtm in a comment. To remove, add /lgtm cancel.
    lgtm label removed on each new commit push.
  • To mark PR as verified comment /verified to the PR, to un-verify comment /verified cancel to the PR.
    verified label removed on each new commit push.
Supported labels

{'/verified', '/wip', '/hold', '/lgtm'}

lugi0
lugi0 previously approved these changes Apr 4, 2025
@fege
Copy link
Copy Markdown
Contributor Author

fege commented Apr 4, 2025

/verified

@opendatahub-tests-bot opendatahub-tests-bot added the Verified Verified pr in Jenkins label Apr 4, 2025
@fege
Copy link
Copy Markdown
Contributor Author

fege commented Apr 4, 2025

/verified

dbasunag
dbasunag previously approved these changes Apr 4, 2025
Copy link
Copy Markdown
Contributor

@rnetser rnetser left a comment

Choose a reason for hiding this comment

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

python 3.9 is supported till 20/2025.
the repo uses
from __future__ import annotations

use typing_extensions for Self

Copy link
Copy Markdown
Contributor

@rnetser rnetser left a comment

Choose a reason for hiding this comment

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

python 3.9 is supported till 20/2025.
the repo uses
from future import annotations

use typing_extensions for Self

@rnetser
Copy link
Copy Markdown
Contributor

rnetser commented Apr 6, 2025

adding as a comment so it is visible in labels:

python 3.9 is supported till 20/2025.
the repo uses
from future import annotations

use typing_extensions for Self

@dbasunag
Copy link
Copy Markdown
Collaborator

dbasunag commented Apr 7, 2025

adding as a comment so it is visible in labels:
python 3.9 is supported till 20/2025.
the repo uses
from future import annotations
use typing_extensions for Self

@rnetser I personally feel the test repo should be using newer python versions, when possible.
from future import annotations should be fine with 3.12. openshift-virtualization uses 3.12 and uses the same import and did not encounter issues.

For use typing_extensions for Self, not sure if we would need it. https://pypi.org/project/typing-extensions/ says it is for older python versions to enable newer typing features and I see Self is here https://github.com/python/cpython/blob/3.12/Lib/typing.py. So I might be missing something, but it does not seem like from typing import Self should be problematic in 3.12

@rnetser
Copy link
Copy Markdown
Contributor

rnetser commented Apr 8, 2025

adding as a comment so it is visible in labels:
python 3.9 is supported till 20/2025.
the repo uses
from future import annotations
use typing_extensions for Self

@rnetser I personally feel the test repo should be using newer python versions, when possible. from future import annotations should be fine with 3.12. openshift-virtualization uses 3.12 and uses the same import and did not encounter issues.

if there's no reason to restrict a version i do not see any reason why limit

For use typing_extensions for Self, not sure if we would need it. https://pypi.org/project/typing-extensions/ says it is for older python versions to enable newer typing features and I see Self is here https://github.com/python/cpython/blob/3.12/Lib/typing.py. So I might be missing something, but it does not seem like from typing import Self should be problematic in 3.12

@fege
Copy link
Copy Markdown
Contributor Author

fege commented Apr 9, 2025

adding as a comment so it is visible in labels:
python 3.9 is supported till 20/2025.
the repo uses
from future import annotations
use typing_extensions for Self

@rnetser I personally feel the test repo should be using newer python versions, when possible. from future import annotations should be fine with 3.12. openshift-virtualization uses 3.12 and uses the same import and did not encounter issues.

if there's no reason to restrict a version i do not see any reason why limit

For use typing_extensions for Self, not sure if we would need it. https://pypi.org/project/typing-extensions/ says it is for older python versions to enable newer typing features and I see Self is here https://github.com/python/cpython/blob/3.12/Lib/typing.py. So I might be missing something, but it does not seem like from typing import Self should be problematic in 3.12

@rnetser are you suggesting to remove completely requires-python ?
I personally think that there is no harm to update to 3.12 given also that this week ended the active support on it https://endoflife.date/python

@rnetser
Copy link
Copy Markdown
Contributor

rnetser commented Apr 9, 2025

adding as a comment so it is visible in labels:
python 3.9 is supported till 20/2025.
the repo uses
from future import annotations
use typing_extensions for Self

@rnetser I personally feel the test repo should be using newer python versions, when possible. from future import annotations should be fine with 3.12. openshift-virtualization uses 3.12 and uses the same import and did not encounter issues.

if there's no reason to restrict a version i do not see any reason why limit

For use typing_extensions for Self, not sure if we would need it. https://pypi.org/project/typing-extensions/ says it is for older python versions to enable newer typing features and I see Self is here https://github.com/python/cpython/blob/3.12/Lib/typing.py. So I might be missing something, but it does not seem like from typing import Self should be problematic in 3.12

@rnetser are you suggesting to remove completely requires-python ? I personally think that there is no harm to update to 3.12 given also that this week ended the active support on it https://endoflife.date/python

I meant leave it on 3.9

@dbasunag
Copy link
Copy Markdown
Collaborator

dbasunag commented Apr 9, 2025

adding as a comment so it is visible in labels:
python 3.9 is supported till 20/2025.
the repo uses
from future import annotations
use typing_extensions for Self

@rnetser I personally feel the test repo should be using newer python versions, when possible. from future import annotations should be fine with 3.12. openshift-virtualization uses 3.12 and uses the same import and did not encounter issues.

if there's no reason to restrict a version i do not see any reason why limit

For use typing_extensions for Self, not sure if we would need it. https://pypi.org/project/typing-extensions/ says it is for older python versions to enable newer typing features and I see Self is here https://github.com/python/cpython/blob/3.12/Lib/typing.py. So I might be missing something, but it does not seem like from typing import Self should be problematic in 3.12

@rnetser are you suggesting to remove completely requires-python ? I personally think that there is no harm to update to 3.12 given also that this week ended the active support on it https://endoflife.date/python

I meant leave it on 3.9

@rnetser can you elaborate why you recommend staying with 3.9? 3.9 active support ended in May 22 and eol is on Oct 2025.

Copy link
Copy Markdown
Collaborator

@dbasunag dbasunag left a comment

Choose a reason for hiding this comment

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

@fege Based on my conversation with @rnetser , lets do the clean up of imports in the same PR.

@fege
Copy link
Copy Markdown
Contributor Author

fege commented Apr 9, 2025

/verified

@opendatahub-tests-bot opendatahub-tests-bot added the Verified Verified pr in Jenkins label Apr 9, 2025
@github-actions github-actions bot added size/xxl and removed size/xl labels Apr 10, 2025
@fege fege requested review from dbasunag, lugi0 and rnetser April 10, 2025 12:32
@fege fege enabled auto-merge (squash) April 11, 2025 08:01
@fege fege merged commit 497cbb0 into opendatahub-io:main Apr 11, 2025
5 checks passed
@fege fege deleted the python_required branch April 11, 2025 08:03
@github-actions
Copy link
Copy Markdown

Status of building tag latest: success.
Status of pushing tag latest to image registry: success.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants