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

Third-party stubs: recommending a default path for installing stub files, overriding stubs #84

Closed
ambv opened this issue Apr 14, 2015 · 47 comments

Comments

@ambv
Copy link
Contributor

ambv commented Apr 14, 2015

The ideal situation is where a file is annotated. The other obvious choice if there is a module.pyi stub file alongside module.py. However, since package maintainers are free not to add type hinting to their packages, there has to be support for third-party stubs installable by pip from PyPI. This opens the following questions:

  • should there be a recommended naming scheme for those stubs? "No, a package maintainer can always trump third-party implementations by including types in his own module.".
  • should there be a default install place for those stubs? "Yes, this makes the type checker's stub discovery much easier. We are proposing using data_files=('share/typehinting/', pathlib.Path(SRC_PATH).glob('**/*.pyi')) in setup.py. We are proposing to add a setup.cfg hook to do the right thing automatically."
  • should third-party stubs be versioned? "Yes, they should be versioned as the lowest version of the source package that they are describing. Example: FooPackage has versions 1.1, 1.2, 1.3, 2.0, 2.1, 2.2, 2.3, 2.4. There are API changes in versions 1.1, 2.0 and 2.4. We need only those three versions of stubs, in which case the user should install the closest lower version of stubs available. If the user lives dangerously and does not pin versions, running both the source package and the stubs as latest will generally work (as long as the stubs are regularly updated)."
  • as mentioned previously, the package itself might provide *.pyi files. There will be a directory (shared/typehinting/) where third-party packages can put their implementation of stubs for external packages. Also, tools like type checkers or IDEs might provide their own custom paths for stubs they themselves populate, etc. The type checker itself is expected to only load one *.pyi file per corresponding *.py module.
ambv added a commit that referenced this issue Apr 17, 2015
@ambv
Copy link
Contributor Author

ambv commented Apr 17, 2015

Ended up recommending shared/typehints/python3.5, etc. since:

  • a different Python version is effectively a diferent environment
  • stub files for the same libraries on a different Python version might be different

@JukkaL, I've seen https://github.com/JukkaL/mypy/tree/master/stubs has a similar concept with distinct stubs per Python version. Do you see any problem with the suggestion?

@JukkaL
Copy link
Contributor

JukkaL commented Apr 17, 2015

My plan is actually to get rid of the separate stub directories for Python 3.4 etc. in mypy. The reason is that it makes stubs a more difficult to maintain with marginal benefits. Having separate stubs for Python 2 and 3 would be useful, however, since they are often significantly different. My plan is to only have stubs/python2 and stubs/python3 (or similar) in mypy.

Also, should the directory be shared/python/typehints/... instead (i.e., with /python/), or maybe shared/pytypehints/...?

Potentially we could recommend something like __minpyversion__ = '3.4' in the stubs to specify the minimum supported Python version.

Also, we discussed the possibility of having a single stub file that works in all Python versions (2.x and 3.x). It would be nice if we didn't have to maintain two copies of such a stub file, as these could easily get out of sync.

@sffjunkie
Copy link

You could do something like the following which allows you to add a more specific stub file for a specific Python version

import os
import sys

class FSUnion():
    def __init__(self, fsroot):
        self._dirs = []
        vi = [str(elem) for elem in sys.version_info[:2]]
        self._dirs.append(os.path.join(fsroot, vi[0], vi[1]))
        self._dirs.append(os.path.join(fsroot, vi[0]))
        self._dirs.append(fsroot)

    def __getitem__(self, module_name:str) -> str:
        for d in self._dirs:
            stub_filename = os.path.join(d, '{}.pyi'.format(module_name))
            if os.path.exists(stub_filename):
                return stub_filename

        raise KeyError('{}: Stub file for {} not found'.format(self.__class__.__name__, module_name))

fsu = FSUnion('typehints')
print(fsu['datetime'])
print(fsu['sys'])
print(fsu['os'])
print(fsu['notthere'])

Which, with the following file structure

typehints
    os.pyi
    sys.pyi
    datetime.pyi
    /3
        os.pyi
        /5
            sys.pyi

prints

typehints\datetime.pyi
typehints\3\5\sys.pyi
typehints\3\os.pyi
Traceback (most recent call last):
  File "fsunion.py", line 26, in <module>
    print(fsu['notthere'])
  File "fsunion.py", line 21, in __getitem__
    raise KeyError('{}: Stub file for {} not found'.format(self.__class__.__name__, module_name))
KeyError: 'FSUnion: Stub file for notthere not found'

@ambv
Copy link
Contributor Author

ambv commented Apr 17, 2015

@JukkaL, so if we want to use PyPI and pip, we have to have pythonX.Y. The reason for that is as follows:

  • a user has python3.3, python3.4 and python3.5 installed on his machine
  • for some reason he doesn't use virtual environments, he is free to ignore them
  • he uses a hypothetical package called "packagify" that since version 2.0 switched to "yield from", and since version 2.1 switched to type annotations in source files
  • if he does pip-3.3 install packagify==1.0 packagify-types==1.0 and then pip-3.4 install packagify==2.0 packagify-types==2.0 and then pip-3.5 install packagify==2.1 (stubs not needed in the last case), he expects types to be valid for each version.

Moreover, as we talked about this with @vlasovskikh, if a construct is described in the stubs, we trust that it is correct. For instance, if functools.pyi has def singledispatch(), then the type checker assumes it exists via some runtime magic, even if it's not there in the source. This would be incorrect for Python 3.3 and a nice bug to catch, actually. So you'd need to have the the hypothetical "stdlib-types" specify "minversion" in functools.pyi. But at this point the stubs stop being usable for Python 3.3 and lower. So you end up introducing versioning in the package name. Back to square one and with more hairy workarounds.

@gvanrossum
Copy link
Member

I can't tell from this discussion if this requires PEP changes or not. I am labeling this as enhancement which I will interpret as "in the future, maybe", i.e. "no need to change the PEP or typing.py now".

@gvanrossum
Copy link
Member

(Sorry, didn't mean to close.)

@o11c
Copy link
Contributor

o11c commented Aug 4, 2015

FYI, I have thought a lot about this, and I think the best path forward is to extend the if typing.PY3 sort of logic.

Currently mypy's stubs DTWT for python 3.2/3.3, because the stubs for modules that were present in 3.2 include functions that were only added in 3.4. And there are even cases where functions were removed in later python versions.

@gvanrossum
Copy link
Member

Actually, Mark Shannon made me remove typing.PY3 and other platform checks. Instead, type checkers should learn how typical programs check for platforms. So extending typing.PY3 is not an option.

(I think this issue is actually about something else, so I won't close it.)

@o11c
Copy link
Contributor

o11c commented Aug 4, 2015

@gvanrossum

In that case, a single stub file can write if sys.version_info[:2] >= (3, 4) to expose different APIs and we can mandate that a checker supports that (as opposed to just a major version).

You're right that it's not the direct focus of this PR, but the choosing in-file vs out-of-file multiversioning will affect the answer for what the paths should be.

That said, I am not looking forward to implementing the "calculate what sys.path would be for a different python version than what we're currently using" logic. But since supporting in-file stubs is the ideal case, we can't avoid that.

@JelleZijlstra
Copy link
Member

JelleZijlstra commented May 25, 2017

I'm interested in moving this issue forward, because it appears to be a somewhat common problem for mypy users that there is no standard place to install third-party stubs outside of typeshed. You can manually install stubs into some directory and set $MYPYPATH, but that is fragile and not portable. By fixing this issue, we could also fix python/typeshed#153, because third-party modules could now come with version-specific stubs.

I think Łukasz's approach of installing stubs using setup(data_files=...) is basically sound, but there's one complication: Type checkers may not have access to the Python binary that is used to run the code they're checking, so they don't know where setup() installed the stubs. I think mypy can get by with something like this for getting 2.7 stubs:

  • Try to run python2.7 -c 'import sys; print(sys.exec_prefix)' and use the output joined with shared/type_hinting/python2.7 as an additional search path for stubs.
  • If that doesn't work (e.g., because the code being checked runs in a virtualenv), it is the user's responsibility to use $MYPYPATH or the mypy_path= config option to teach mypy to find the stubs.
  • Perhaps mypy could also provide an option like --check-using-python /path/to/python/binary. If this is given, it can query the binary for its exec_prefix and find the stubs that way.

Other type checkers may provide additional implementation-specific ways to find the shared stub directory.

If we go with this approach, how should it be codified? I could write a new PEP, but perhaps this is small enough that it can just go as a new section into PEP 484.

I have a proof-of-concept implementation in https://github.com/JelleZijlstra/mypy/tree/stubdir and a library using the functionality at https://github.com/JelleZijlstra/sqlalchemy-stubs.

cc @JukkaL @vlasovskikh @matthiaskramm

JelleZijlstra added a commit to JelleZijlstra/sqlalchemy-stubs that referenced this issue May 25, 2017
Using functionality proposed in python/typing#84
@matthiaskramm
Copy link
Contributor

I like this approach, but I'd propose using the same naming scheme for type-hinting that we have in typeshed.
(I.e., use type-hinting/2.7, not type-hinting/python2.7)

@gvanrossum
Copy link
Member

The PEP 484 text on this issue is pretty vague, and I propose to write a separate PEP, just so the design is clear and we can easily get feedback from people maintaining various libraries or using various platforms. Can you start a PEP for this purpose?

@JelleZijlstra
Copy link
Member

Yes, I'll do that.

@JelleZijlstra
Copy link
Member

Actually, I just re-read PEP 484, and at https://www.python.org/dev/peps/pep-0484/#storing-and-distributing-stub-files it has pretty much the same specification that's being proposed here. However, that text seems to implicitly assume type checkers run the same Python version they're checking, which is not currently true. So there's still value in a new PEP to specify in more detail how stub files should be distributed.

I'm going to work on the new PEP at https://github.com/JelleZijlstra/peps/tree/distributingstubs. I'll post here again to ask for feedback when I'm happy with the PEP text as written, but in the meantime I'm open to any suggestions on what the PEP should cover and how the system should work.

@gvanrossum
Copy link
Member

Great! I feel that the current text in the PEP falls short in giving an algorithm for a type checker for finding stubs, even given a Python executable (to which it can feed arbitrary code, within reason). The PEP currently defers to PYTHONPATH (and where is the shared directory rooted?).

@ethanhs
Copy link
Contributor

ethanhs commented May 27, 2017

This is something I think would be really important to have! I was considering writing up a proposal myself. If you need help with the PEP, I'd be happy to help in reviewing or any other way I can!

@ambv
Copy link
Contributor Author

ambv commented Jul 13, 2017

Mypy documentation is currently warning against using site-packages with MYPYPATH. Rightfully so, in this case it's also type checking all third-party code your application is importing, which the user most likely doesn't want. Example: add site-packages to your MYPYPATH and just import setuptools somewhere in your code to see tens of confusing errors.

However, the more I think about it in context of shipping annotated libraries or .pyi files for user consumption, the more I think we should just use site-packages. Why?

  1. For the user, it's the obvious place to look for annotated code and stubs, if they are provided by the author.
  2. For the author, the default action of just submitting a package to PyPI is enough to make it available to type checkers. It does the correct thing by default.

There are three issues with this that I can identify:

  1. Third-party libraries which aren't annotated at all often generate internal type errors.
  2. Third-party libraries which are annotated sometimes still generate internal type errors.
  3. Shipping .pyi files for production libraries is wasting space.

The first two can be solved by teaching the type checker that errors in site-packages should be silenced by default unless really asked for. Essentially a less hacky form of this: MYPYPATH=.../lib/python3.6/site-packages mypy my_project | grep -v "/site-packages/"

As an option we can consider also ignoring .py files altogether if they don't contain a single type annotation/type comment.

The size aspect of a library installing .pyi files is a nit. Embedded type annotations already have weight, so moving them to a separate .pyi file is a miniscule cost. If the author of a given library really cares about every last byte, we can recommend putting typing as a separate extra feature in setup.py (so the user will have to pip install some_library[typing] to get it).

cc @JelleZijlstra

@gvanrossum
Copy link
Member

Mypy has a feature --follow-imports=silent which lets it analyze code that is needed to satisfy imports but not specified on the command line. This is indeed just a flag to suppress errors from those source files. However what's also needed is a way to decide whether to use the site-packages code for a given library or the typeshed stubs, if the latter exist. I think one of the problems with the way mypy currently interprets MYPYPATH is that everything it finds there has higher priority than typeshed. But (except in rare cases) when stubs exist for a 3rd party module, mypy should prefer those stubs over the code in site-packages. Hopefully this can be solved through some kind of improvement to mypy's search path -- maybe MYPYPATH can include a token indicating where typeshed should be searched, so you could write e.g. MYPYPATH=blah:blahblah:<typeshed>:/path/to/site-packages.

I think it would also be good to be able to point a type checker to a particular module in site-packages without implying that site-packages itself should go on the search path (this is probably one of the less-understood features of mypy's search path -- for any file or folder specified on the command line it implicitly adds the containing folder to the search path, before MYPYPATH).

@ethanhs
Copy link
Contributor

ethanhs commented Jul 13, 2017

I agree that people should be able to point to a particular module in site packages, instead of all of it. Im concerned if we start trying to deal with files that are untyped and not verified against Mypy it may crash, which would obviously be a problem, as the user would have to uninstall or modify the installed library, which isn't a great experience either way.

@gvanrossum
Copy link
Member

Yes, for example I had a spurious install of typing.py in my site-packages and that totally crashed mypy.

I propose a concrete test case: there are stubs in typeshed for werkzeug and jinja2 but not for flask. It should be possible to typecheck site-packages/flask while using the typeshed stubs for werkzeug and jinja2. Currently the only way I can think of making that work is making a copy of the flask package (or a symlink or take the source tree) and point mypy at that -- but I think it should be possible to just do it using mypy $VIRTUAL_ENV/lib/python3.6/site-packages/flask or, even better, mypy -m flask.

@asvetlov
Copy link

asvetlov commented Jul 26, 2017

I don't think versioning for stub libraries is an issue: user should install them by pip install, he could pin stub version together with base library, e.g. in requirement files etc.

Though we could recommend supporting the same version number by stub library authors, e.g. for Django==1.11 django-stub should be 1.11 too.

@JelleZijlstra
Copy link
Member

But what if the author django-stub discovers that they made a mistake in their stubs for Django 1.11? Perhaps they could release django-stub 1.11.1, but then that would mean they'd have to maintain separate, mostly identical copies of the stubs for each Django version.

I think we'll have to support something like this straw man: stubs can do if __version__ >= (1, 11), where __version__ is a magical constant that the type checker evaluates to the version of the package that is being used. There's many details there that I haven't thought much about: what about namespace packages? how does the type checker know what library version you are using?

@gvanrossum
Copy link
Member

gvanrossum commented Jul 27, 2017

Yeah, I think this proposal supports library versioning well enough.

[Clarification: This was written before Jelle's comment above, in response to @asvetlov's "I don't think versioning for stub libraries is an issue".]

@gvanrossum
Copy link
Member

But what if the author django-stub discovers that they made a mistake in their stubs for Django 1.11? Perhaps they could release django-stub 1.11.1, but then that would mean they'd have to maintain separate, mostly identical copies of the stubs for each Django version.

I think we'll have to support something like this straw man: stubs can do if __version__ >= (1, 11), where __version__ is a magical constant that the type checker evaluates to the version of the package that is being used. There's many details there that I haven't thought much about: what about namespace packages? how does the type checker know what library version you are using?

This feels like worrying too much. We've never even tried any of this proposal, and we don't know if this scenario will occur frequently enough to worry about. But if we specify something to handle this now we'll never be able to remove that feature, no matter how unpopular (because it'll always break someone's workflow).

Also, I really don't like having to support library version comparisons in the checker -- unlike Python versions we don't have control over the ordering of libraries (they don't all use semver). The maintenance problem can be solved using branches. We're better off allowing some freedom in the naming of stub packages -- maybe the package name will end up including the django version (e.g. django-1.1-stubs) so the package version can be whatever the stub package author wants.

@ethanhs
Copy link
Contributor

ethanhs commented Sep 9, 2017

I've started writing up a PEP. Hopefully I can get a draft out tomorrow or the next day.

Things to think about which I leave open for discussion (because they should be discussed more):

  • How should packages indicate they support typing? (My personal favorite is a new trove classifier, but other options exist)
  • How should mixed stub/inline packages be dealt with? If we have stubs in a package, should we parse the Python files to check if there is type information in the files? That could be slow for large codebases. Should we have 3 states of metadata? Stubs, inline, mixed? That has its own drawbacks.
  • What to do with the existing PEP 484 text on the matter? Some say we should delete it and replace it with the new PEP. Im not sure that is the best thing to do, but it sounds cleaner.
  • Other issues with current designs.

@gvanrossum
Copy link
Member

  • I'm not sure I like Trove classifiers that much. They're almost free-form but here we need something machine parseable. Maybe there's something in PEP 459? Or maybe we can just add something else to the egg-info (but I don't know how any of that stuff works, really).

  • For packages that declare they support typing, we should use the standard search approach, which is .pyi first then .py, ignoring the .py if the .pyi is found.

@ethanhs
Copy link
Contributor

ethanhs commented Sep 10, 2017

Okay, I have a rough draft here: https://github.com/ethanhs/peps/blob/typedist/pep-0561.rst

I also added a PR python/peps#415 if people prefer the Github review UI, but also feel free to leave comments in this issue.

I plan on making a POC implementation of the distutils extension when I have time either later today or tomorrow.

A couple points Im not sure about:

should the keyword be a boolean about whether the package is stubbed or not? Then stubbed == True inline == False and type unsafe == None.

Mixed inline/stubbed packages - I originally wrote a version with an additional option for this, but it seemed to make things more complicated than needed.

Thanks!

@ethanhs
Copy link
Contributor

ethanhs commented Oct 27, 2017

The PEP has been posted to Python-dev and the latest live version can be found here: https://www.python.org/dev/peps/pep-0561/

chadrik added a commit to chadrik/attrs that referenced this issue Nov 3, 2017
This is the recommended approach for 3rd party stubs.
See: python/typing#84 (comment)
hynek pushed a commit to python-attrs/attrs that referenced this issue Jul 12, 2018
* Add PEP484 stubs

* Deploy .pyi stubs alongside .py files.

This is the recommended approach for 3rd party stubs.
See: python/typing#84 (comment)

* Add support for the new type argument.

* Add tests for stubs and address a few issues.

* Improve declaration of private vs public objects in stubs

* More stub tests

* Separate the stub tests into their own tox env

it does not make sense to test the stubs in multiple python *runtime* environments (e.g. python 3.5, 3.6, pypy3) because the results of static analysis wrt attrs is not dependent on the runtime.  Moreover, mypy is not installing correctly in pypy3 which has nothing to do with attrs.

* Update the manifest with stub files

* Remove mypy from the dev requirements

* Allow _CountingAttr to be instantiated, but not Attribute.

* Incorporate defaults into attr.ib typing

* Fix a bug with validators.and_

* Add more tests

* Remove _CountingAttr from public interface

It is crucial to ensure that make_class() works with attr.ib(), as a result we no longer have any functions that care about _CountingAttr.

* Lie about return type of Factory

this allows for an abbreviated idiom: `x: List[int] = Factory(list)`

* Add tox stubs env to travis

* used the wrong comment character in mypy tests

* Improve overloads using PyCharm order-based approach

overloads are pretty broken in mypy.  the best we can do for now is target PyCharm, which is much more forgiving.

* Remove features not yet working in mypy. Document remaining issues.

* Test stubs against euresti fork of mypy with attrs plugin

Copied the pytest plugin from mypy for testing annotations: It is not an officially supported API and using the plugin from mypy could break after any update.

* Add some types and TypeVars to some types. Make tests pass

* Suppress warnings about named attribute access from fields()

e.g.  fields(C).x
Eventually it would be good to add support for returning NamedTuple from the mypy plugin

* Add WIP mypy-doctest plugin

* Deal with a few remaining type issues in the docs

* sphinx doctest: don't turn warnings into errors.

doing so makes the tests abort after the first failed group.

* Update "type: ignore" comments to reflect issues fixed in mypy plugin

* doctest2: improve output formatting

* Update manifest

* static tests: use inline error declarations

* More tests

* Tests passing (with notes about remaining issues)

* Attempt to get latest plugin from euresti working

* Issues fixed.

Had to place calls to attr.ib under a class definition.

* Deal with a PyCharm bug

* Minor test improvements

* Make tests prettier

* Use 2 decorators instead of 3

* doctest2: add support for skipping mypy tests

* Add tests for inheritance, eq, and cmp

* Add fixmes and todos

* Rename convert to converter

* Attribute.validator is always a single validator

* Conform stubs to typeshed coding style

And add auto_attrib kw

* backport style fixes from typeshed

* Add test cases to cover forward references and Any

* Add fixes for forward references and Any

* Address typeshed review notes

* Use Sequence instead of List/Tuple for validator arg

list and tuple are invariant and so prevent passing subtypes of _ValidatorType

* backports changes from typeshed #1914

* backport changes from typeshed #1933

* Prevent mypy tests from getting picked up

Evidently the discovery rules changed recently for pytest.

* make our doctest extension compatible with latest sphinx

* Adjustments to the tests

* Fix flake and manifest tests (hopefully)

* Fix tests on pypy3 (hopefully)

* Update stubs from typeshed

Also update tests.

* Make PEP 561-compliant

* minor cleanup

* Consolidate stub support files into stub directory

In preparation for removing them from the pyi_stubs branch.

* Get tests passing

This is a final test of the current stubs before moving the stub tests to a new branch.

* Revert stub test additions

Replace with a simple mypy pass/fail test

* get pre-commit passing

* Address review feedback

* Move typing test up in tox envlist
@ethanhs
Copy link
Contributor

ethanhs commented Jul 26, 2018

I believe PEP 561 resolves this issue, now that it is accepted.

@gvanrossum
Copy link
Member

Yes!

@ilevkivskyi
Copy link
Member

I believe PEP 561 resolves this issue, now that it is accepted.

It was a long way. Thanks @ethanhs for all the work on this!

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

No branches or pull requests