Skip to content

typing: move from comment type hints to Python 3.6 hints#34305

Merged
tgamblin merged 1 commit intodevelopfrom
fix-typing
Dec 5, 2022
Merged

typing: move from comment type hints to Python 3.6 hints#34305
tgamblin merged 1 commit intodevelopfrom
fix-typing

Conversation

@tgamblin
Copy link
Copy Markdown
Member

@tgamblin tgamblin commented Dec 4, 2022

We've stopped supporting Python 2, and contributors are noticing that our CI no longer allows Python 2.7 comment type hints. They end up having to adapt them, but this adds extra unrelated work to PRs.

  • Move to 3.6 type hints across the entire code base

adamjstewart
adamjstewart previously approved these changes Dec 4, 2022
@tgamblin
Copy link
Copy Markdown
Member Author

tgamblin commented Dec 4, 2022

Sigh. TIL about typing.Match. Match objects have different types in 3.6 vs. 3.7+:

Python 3.6.15 (default, Dec 21 2021, 08:03:21)
[GCC 10.2.1 20210110] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> import re
>>> re.match("foo", "foo")
<_sre.SRE_Match object; span=(0, 3), match='foo'>
Python 3.7.15 (default, Nov 15 2022, 10:33:03)
[GCC 10.2.1 20210110] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> import re
>>> re.match("foo", "foo")
<re.Match object; span=(0, 3), match='foo'>

So you have to use typing.Match[str] in type hints.

adamjstewart
adamjstewart previously approved these changes Dec 4, 2022
@adamjstewart
Copy link
Copy Markdown
Member

Are we running the style tests on multiple versions of Python? I think we only need to test on the latest version.

adamjstewart
adamjstewart previously approved these changes Dec 4, 2022
@tgamblin
Copy link
Copy Markdown
Member Author

tgamblin commented Dec 5, 2022

@adamjstewart:

Are we running the style tests on multiple versions of Python? I think we only need to test on the latest version.

When you use the real type hints, things can fail in older versions even if we do not run the style checks. e.g.:

import re
from typing import List, Tuple

    def __init__(self, all_unquoted_flag_pairs: List[Tuple[re.Match, str]]):
        ...

re.Match is used in code that needs to work at class definition time for every version we support, but it does not exist in 3.6. The code above should be:

import re
from typing import List, Match, Tuple

    def __init__(self, all_unquoted_flag_pairs: List[Tuple[Match[str], str]]):
        ...

@tgamblin tgamblin changed the title typing: move from comment annotations to Python 3.6 annotations typing: move from comment type hints to Python 3.6 hints Dec 5, 2022
We've stopped supporting Python 2, and contributors are noticing that our CI no longer
allows Python 2.7 comment type hints. They end up having to adapt them, but this adds
extra unrelated work to PRs.

- [x] Move to 3.6 type hints across the entire code base
@adamjstewart
Copy link
Copy Markdown
Member

Gotcha. There's also:

from __future__ import annotations

but I've been avoiding that because of https://docs.python.org/3/library/__future__.html#id2.

@tgamblin
Copy link
Copy Markdown
Member Author

tgamblin commented Dec 5, 2022

Sadly looks like we cannot have that until 3.7

@tgamblin tgamblin merged commit 82b7fe6 into develop Dec 5, 2022
@tgamblin tgamblin deleted the fix-typing branch December 5, 2022 05:41
luke-dt pushed a commit to dantaslab/spack that referenced this pull request Dec 5, 2022
…k#34305)

We've stopped supporting Python 2, and contributors are noticing that our CI no longer
allows Python 2.7 comment type hints. They end up having to adapt them, but this adds
extra unrelated work to PRs.

- [x] Move to 3.6 type hints across the entire code base
tgamblin added a commit that referenced this pull request Dec 23, 2022
tgamblin added a commit that referenced this pull request Dec 23, 2022
haampie pushed a commit that referenced this pull request Dec 26, 2022
stephenmsachs pushed a commit to stephenmsachs/spack that referenced this pull request Jan 3, 2023
RikkiButler20 pushed a commit to RikkiButler20/spack that referenced this pull request Jan 24, 2023
amd-toolchain-support pushed a commit to amd-toolchain-support/spack that referenced this pull request Feb 16, 2023
…k#34305)

We've stopped supporting Python 2, and contributors are noticing that our CI no longer
allows Python 2.7 comment type hints. They end up having to adapt them, but this adds
extra unrelated work to PRs.

- [x] Move to 3.6 type hints across the entire code base
amd-toolchain-support pushed a commit to amd-toolchain-support/spack that referenced this pull request Feb 16, 2023
techxdave pushed a commit to Tech-XCorp/spack that referenced this pull request Feb 17, 2023
jmcarcell pushed a commit to key4hep/spack that referenced this pull request Apr 13, 2023
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.

2 participants