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

Should we remove the stubs from typeshed? #480

Closed
euresti opened this issue Dec 27, 2018 · 11 comments
Closed

Should we remove the stubs from typeshed? #480

euresti opened this issue Dec 27, 2018 · 11 comments
Labels

Comments

@euresti
Copy link
Contributor

euresti commented Dec 27, 2018

Now that mypy supports pep 561 (python/mypy#4693) and attrs includes type stubs (#238) should we remove the type annotations that are in typeshed? This way we don't have to keep those in sync with attrs development.

@ethanhs
Copy link
Member

ethanhs commented Dec 27, 2018

The only down side I see with this is that other type checkers do not support PEP 561 yet. PyCharm has support for it now, but both pytype and pyre do not. Otherwise, I think this would simplify things nicely.

If stubs are pulled out of typeshed, perhaps it also makes sense to move the attrs mypy plugin into the attrs source tree as well?

@hynek
Copy link
Member

hynek commented Dec 28, 2018

I guess this comes ultimately down to mypy’s devs decision. I’m of course all for cleaning stuff up.

@euresti
Copy link
Contributor Author

euresti commented Dec 29, 2018

I'm hesitant to move the plugin into just attrs because that way they run our tests and a breaking change will have to be fixed before it gets merged into mypy.

@hynek I'm not sure what you mean by "mypy devs decision".

That being said I've made a PR on typeshed to update the stubs. I think we should probably wait until pytype and pyre support Pep 561.

@hynek
Copy link
Member

hynek commented Dec 30, 2018

I meant that whether or not the stubs are removed isn’t/shouldn’t be our decision. 🤷‍♂️

If it weren’t for pytype/pyre I’d even argue to not update those stubs at all since they’d be used by people who run older attrs.

@ilevkivskyi
Copy link

In general, I think this is a good idea in long term perspective. Having a single package that incorporates PEP 561 stubs and a mypy plugin is a simple way to keep all three in sync. A possible downside is that we need to be sure that the plugin works well with more than one mypy version, since currently plugins API is quickly changing.

Also if we are going to do this, we will also need to migrate the open issues.

@hynek
Copy link
Member

hynek commented Mar 30, 2020

Since the big moment of starting to work on import attrs is coming closer, I think we should re-evaluate this. Having to pester the mypy devs with every change we make to our APIs isn't great for both sides.


@euresti since you basically wrote the plugin, what is your current position? Could be pull this off?

@euresti
Copy link
Contributor Author

euresti commented Apr 1, 2020

There are 2 pieces:
A) stubs

  • These are currently in attrs and sadly there is an old version in typeshed
  • Pytype and Pyre still don't support it
    Support PEP 561? facebook/pyre-check#70
    Support PEP 561 (Distributing and Packaging Type Information) google/pytype#151
  • Updating the stubs in typeshed is relatively simple (it's an easy PR) but the fact that they go out of sync can make things complicated.
  • So if we think that pytype and pyre are not that used then I can make a PR to delete them from typeshed and be happy.
  • The only breakages I can think of is that if you are using a separate venv for mypy then you now have to install attrs in that venv in order to typecheck correctly.

B) plugin
Being absolutely honest I prefer to leave the plugin as part of mypy for the following reasons:

  • If someone modifies something that breaks the plugin tests they'll have to go and figure out how to fix it.
  • If someone submits a PR there will be mypy devs looking at it who have way more knowledge than we do about how mypy works.
  • The one obvious annoyance is the version skew. I can't decide if it's a necessary evil or if it's the straw that breaks the camel's back. I will note that the version skew could happen in either situation. Maybe we move the plugin to attrs and then a new version of mypy breaks our plugin in a past version of attrs.

@hynek
Copy link
Member

hynek commented Apr 2, 2020

Thanks for your thoughts!

TBH, I don't think about stubs at all anymore. We're shipping them forever and I don't see it as my job to keep others updated.

As for the plugin, I'm gonna 100% follow your recommendations here because I'm not able to maintain it. If at some point someone steps up and want to take ownership of it, we can talk again but I guess that matter is settled for now.

@hynek hynek closed this as completed Apr 2, 2020
@euresti
Copy link
Contributor Author

euresti commented Apr 2, 2020

So on the question of the stubs. Should I make a PR to remove them from typeshed?

@hynek
Copy link
Member

hynek commented Apr 2, 2020

I honestly don't care. I'd leave that to the mypy devs since it doesn't affect us. If someone wants to volunteer and keep our stubs synced that's cool too. I just don't want to see any tickets on our tracker.

@wsanchez
Copy link

wsanchez commented Apr 2, 2020

I would say yes to removing them from typeshed. If pytype and pyre users want them, those projects should get fixed, right? If not… nobody cares.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

5 participants