Skip to content

Conversation

@srittau
Copy link
Collaborator

@srittau srittau commented May 14, 2021

All new files in stdlib/@python2 are straight copies of the corresponding files in stdlib. Edit: With one change: genericpath.pyi gained a new # type: ignore, since mypy started to complain about it. Edit 2: I also updated CONTRIBUTING.md.

I know that this a controversial change, which is why I marked this PR as a draft for now as I would like to reach a consensus before merging this. Here's my reasoning (which is partly a repeat of the arguments I made in #5049):

Let me prefix this that I personally haven't worked with Python 2 for quite a while and never with typing support, so that my reasoning could make some very wrong assumptions. But I think that Python 2 and 3 stubs have quite different requirements:

  • Python 2 stubs are meant for legacy code that rarely changes. Type annotations for this kind of code are often a first step towards moving to Python 3. Also, the Python 2 standard library is fixed and won't change. Considering we only rarely get PRs that target Python 2 specifically, the Python 2 stubs seem to be mostly "good enough". Also, while we can use some new typing features in Python 2 stubs, most are reserved for Python 3.
  • Python 3 stubs have a moving target as the standard library constantly evolves, but also because typing gains new features we can use to improve the stubs. Also, while Python 3 can be legacy code as well, often Python 3 code is under active development and is better equipped to deal with evolving stubs.

These are the background why I think splitting the stubs makes sense:

  • Python 2 code is not affected by accidental changes made to benefit Python 3, but are harmful to Python 2, for example using str instead of Text or using typing features not available for Python 2.
  • We have less tests in place to ensure the correctness of Python 2 stubs. For example, I am not sure if mypy_primer supports Python 2. (Cc @hauntsaninja) But even if it does, there is less and less code out in the wild supporting it to test against.
  • On the other hand, Python 3 stubs can often benefit from newer typing features. It's also easier to drop support for unsupported Python 3 versions that for Python 2, so newer syntax (e.g. positional-only syntax) can become available for Python 3 stubs that's not available for Python 2.
  • We don't need to support Python 2 kludges (e.g. Text) and Python 2 branches in Python 3 stubs, making the stubs clearer and easier to maintain in general.
  • Currently our structure is a bit confusing for new contributors, but also cumbersome for existing ones. For example, when making a change to a stub, I have to double-check whether this stub also support by Python 2 by looking into VERSIONS and the @python2 directory.

Finally, by doing this "big split" once and without any other changes has the advantage of clarity in the commit history over our current practice of splitting ad-hoc when changes are made for a stub.

See also #5049 for previous discussion.


This is the script I used to split the stubs:

#!/usr/bin/env python3

from pathlib import Path
from shutil import copyfile, copytree

py2_modules = []
with open("stdlib/VERSIONS") as f:
    for line in f:
        line = line.split("#")[0].strip()
        if not line:
            continue
        module, version = line.split(":")
        if version.strip().startswith("2.7-"):
            py2_modules.append(module)

for module in py2_modules:
    src_dir = Path("stdlib", module)
    src_file = Path("stdlib", f"{module}.pyi")
    py2_dir = Path("stdlib", "@python2", module)
    py2_file = Path("stdlib", "@python2", f"{module}.pyi")
    if py2_dir.exists() or py2_file.exists():
        continue
    if src_dir.exists():
        copytree(src_dir, py2_dir)
    elif src_file.exists():
        copyfile(src_file, py2_file)
    else:
        raise ValueError(f"missing src module '{module}'")

All new files in stdlib/@python2 are straight copies of the
corresponding files in stdlib.
@srittau
Copy link
Collaborator Author

srittau commented May 14, 2021

One final thought: This could make type checker logic for where to look for Python 2 stubs a bit easier, as they don't even need to look at VERSIONS when checking Python 2 code. Currently they need to look into VERSIONS, but also check the @python2 directory.

srittau added a commit to srittau/typeshed that referenced this pull request May 14, 2021
It was called ConfigParser in Python 2.

Split out from python#5442.
@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

4 similar comments
@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

Copy link
Contributor

@JukkaL JukkaL left a comment

Choose a reason for hiding this comment

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

Overall I like this idea. I've seen changes to combined Python 2/3 stubs break them on Python 2 because somebody forgot to use Text, and this should help with that, in particular. I also agree with the other motivating factors.

Afterwards, we could also replace Text types in Python 3 stubs with str, making the stubs a bit cleaner.

print_function: _Feature
unicode_literals: _Feature
with_statement: _Feature
if sys.version_info >= (3, 0):
Copy link
Contributor

Choose a reason for hiding this comment

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

These Python 3 version checks might be worth removing at some point, at least if we can script it easily.

JelleZijlstra pushed a commit that referenced this pull request May 14, 2021
It was called ConfigParser in Python 2.

Split out from #5442.
@JelleZijlstra
Copy link
Member

I'm on board with doing this now. #5445 is another example of how the Python 2 stubs are hard to maintain correctly if we don't keep them separate.

Hopefully in a couple of years we can drop Python 2 stubs completely.

@github-actions

This comment has been minimized.

@Akuli
Copy link
Collaborator

Akuli commented May 14, 2021

I generally don't like pull requests that add a lot of code, but I see the rationale and I think this is a good idea. The usual arguments for why copy/pasta is bad don't really apply here, as the intent is to not port every fix to Python 2.

@srittau srittau marked this pull request as ready for review May 14, 2021 13:53
@github-actions
Copy link
Contributor

According to mypy_primer, this change has no effect on the checked open source code. 🤖🎉

@Akuli
Copy link
Collaborator

Akuli commented May 14, 2021

Before merging, I think we should ask what other active maintainers think about this, e.g. @hauntsaninja and @ilevkivskyi.

@ilevkivskyi
Copy link
Member

LGTM. You may double-check auto-uploader will be not broken by this (although I am 99% sure it shouldn't).

@srittau
Copy link
Collaborator Author

srittau commented May 14, 2021

Since this is only about the stdlib at the moment, it shouldn't affect the auto-uploader. That said, we could think about doing this to third-party libs as well, but I don't think it's as important.

@hauntsaninja
Copy link
Collaborator

I'm fine with this.
(It's possible mypy_primer might have one or two projects that typecheck with Python 2, but it would be by accident rather than intentional on my part. stubtest definitely does not support Python 2)

@srittau
Copy link
Collaborator Author

srittau commented May 14, 2021

All maintainers are in favor, tests pass, if no one beats me to it, I'm going to merge it in a bit.

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.

6 participants