Skip to content
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

Adding platform information to stdlib/VERSIONS #11260

Open
srittau opened this issue Jan 8, 2024 · 12 comments
Open

Adding platform information to stdlib/VERSIONS #11260

srittau opened this issue Jan 8, 2024 · 12 comments
Labels
project: policy Organization of the typeshed project

Comments

@srittau
Copy link
Collaborator

srittau commented Jan 8, 2024

It might be a good idea to add platform availability information to stdlib/VERSIONS, instead of using the sys.platform hack we're currently using in the source files. This has the same rationale for introducing VERSIONS in the first place:

  • Remove platform checks and the required indentation from stub files.
  • Enables clearer type checker messages (e.g. "Module does not exist on this platform", instead of "Item X doesn't exist in module Y")
  • Generally an unambiguous way of indicating that a module doesn't exist.
  • Edit: Collect all information about module availability in one place.

One way to do this is an HTTP/E-Mail header-like key/value syntax:

abc: 3.0-; platforms=linux,darwin
def: 3.7-3.10; platforms=!windows,!linux
ghi: 3.9-

This would allow future extension when type checkers are equipped to ignore unknown keys:

abc: 3.0-; header1=foo; header2=bar

Do we need a way do indicate that a certain platform only supports a certain module starting from a certain Linux version?

@srittau srittau added the project: policy Organization of the typeshed project label Jan 8, 2024
@srittau
Copy link
Collaborator Author

srittau commented Jan 8, 2024

@erictraut
Copy link
Contributor

Is there a concrete reason for doing this now, or is this this just theoretical? If it's just theoretical, I'd prefer to hold off until there's a proven need for this facility.

@srittau
Copy link
Collaborator Author

srittau commented Jan 8, 2024

The need is the same as for initially adding VERSIONS: Addressing a deficiency in how type checkers can detect whether stubs are available in a certain Python version. But the concrete trigger was #6749/#11241: Having a solid way to say "This package doesn't exist on this platform in the stdlib" would enable a third-party stubs package to reliably add that package without interfering with a stdlib package.

@srittau
Copy link
Collaborator Author

srittau commented Jan 8, 2024

Also just to clarify: There would be no need to immediately implement this on the type checker side and there is also no pressing need on typeshed's side to remove the version_info checks when this gets implemented. This is as possible timeline:

  1. We find consensus on a format - ideally one that's future-compatible so we don't need immediate changes the next time the VERSIONS format is amended.
  2. Type checkers either implement the format or skip it for now (for example by implementing the equivalent of line = line.split(";")[0]).
  3. We add platform information to the VERSIONS file.

Then type checkers have the option to implement the new-style checks. I expect typeshed to keep the current in-stub checks for at least as long as the major type checkers haven't implemented the new-style checks.

@erictraut
Copy link
Contributor

OK, makes sense. I've added provisional support for the above proposal in pyright. If we end up modifying the proposal, I will adjust accordingly.

I presume that if there are one or more platforms listed without ! characters, that all other platforms are assumed to be unsupported.

erictraut added a commit to microsoft/pyright that referenced this issue Jan 9, 2024
@Akuli
Copy link
Collaborator

Akuli commented Jan 10, 2024

I'm not convinced that this is a good idea. Currently the syntax of the VERSIONS file is super simple, and I'd prefer to leave it that way instead of inventing a complex language that is only used to describe one file.

Also, the VERSIONS file is supposed to contain info about standard library specific things. To me, "Python version added" is clearly a standard-library thing while "supported platforms" could apply to any stub file. So if we do this for standard-library stubs, it would be tempting to do something similar for third-party stubs, which would introduce even more complexity for little benefit.

To me, the only convincing reason is better type checker error messages, because users would see it.

@srittau
Copy link
Collaborator Author

srittau commented Jan 10, 2024

To me, "Python version added" is clearly a standard-library thing while "supported platforms" could apply to any stub file.

That's a good point. A generic way to tell type checkers "this module is not available on that platform" would be preferable. That said, amending VERSIONS is better than the status quo.

I don't think keeping VERSIONS super simple should be a design goal, as it's not user facing. Also, to me, the proposed format is simple enough, and only used for a few modules.

@Akuli
Copy link
Collaborator

Akuli commented Jan 21, 2024

A generic way to tell type checkers "this module is not available on that platform" would be preferable.

A bit weird, but maybe type checkers could just check if everything in the stub file (except imports) is defined inside a if sys.platform == statement that is being skipped?

Advantages:

  • No new syntax needed
  • No more inventing non-standard file formats
  • Readable
  • Super simple

Disadvantages:

  • Adding just one public symbol to the wrong place flags an entire module as "available on all platforms" (would need CI check)
  • Doesn't feel very reliable, perhaps it is too magical for the curses situation

@rchen152
Copy link
Collaborator

Echoing @Akuli, adding platform information to stdlib/VERSIONS feels like a bit of an awkward fit, since it's not stdlib-specific. A couple other ideas off the top of my head:

  • We could put platform information in a separate PLATFORMS file. The stdlib could have one, and any stub distribution that needs to communicate platform info could add its own.
  • Platform info could be indicated in stubs themselves with something like assert sys.platform != "windows" at the top. (I think I've seen this suggestion go by before, so apologies if it's already been discussed and rejected.)

@srittau
Copy link
Collaborator Author

srittau commented Jan 24, 2024

Ideally, both platform and Python version availability would be encoded in the stub file itself instead of an external file. But for now I think Rebecca's suggestion to use a PLATFORMS file sounds like the best idea, as it's named unambiguously (as opposed to VERSIONS) and backwards-compatible.

@JelleZijlstra
Copy link
Member

JelleZijlstra commented Jan 24, 2024

I think I'd prefer to implement Rebecca's second suggestion. This would not require a new special file format, and it would generalize to third-party packages more easily.

Suggested specification: The first line of code in a stub file may be an assert statement. It may contain the same set of conditions as are supported in if statements in stubs (checks on sys.platform and sys.version_info). If this assert is present, type checkers should evaluate it and if the condition statically evaluates to false, they should treat the stub file as if it does not exist.

@erictraut
Copy link
Contributor

erictraut commented Jan 24, 2024

The first line of code in a stub file may be an assert statement

That won't work for pyright and pylance. We need to determine which type stubs are applicable without loading, parsing and analyzing every type stub in the environment. This is important for language server features like completion suggestions with auto-import.

A static file like PLATFORMS or an extension to VERSIONS would work much better for our use case.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
project: policy Organization of the typeshed project
Projects
None yet
Development

No branches or pull requests

5 participants