Skip to content

Create recommended process for notifying 3rd party package maintainers #8

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

Closed
JukkaL opened this issue Apr 13, 2015 · 12 comments
Closed

Comments

@JukkaL
Copy link
Contributor

JukkaL commented Apr 13, 2015

No description provided.

@Lukasa
Copy link

Lukasa commented Nov 17, 2015

Can I suggest that this should be worked on before accepting further third-party stubs? As of right now I count more than 19 third-party type stubs, including requests (twice), of which I am one of the maintainers. I am not aware that any requests maintainer was contacted about type stubs being added for requests, despite the claims in PEP 484 (emphasis mine):

Note that stubs for a given package will not be included here without the explicit consent of the package owner.

It's possible that I'm wrong and that one of the other maintainers was contacted, but it's my suspicion that we weren't. Given this, I suspect many, possibly all, of the third-party stubs are in the same boat. There's also been some troubling communication from the typeshed maintainers on this topic here:

As long as the package owner is fine with you submitting stubs (and we'll just take your word for that)...

This sentiment seems to be in complete opposition to PEP 484, which requires explicit consent: taking the submitters word for it is the very definition of implicit consent.

To be clear, the requests project has no objection to type stubs being submitted to the typeshed, but other maintainers may object. I recommend putting this process in place now, rather than wait for this to blow up into something.

@sigmavirus24
Copy link

the requests project has no objection to type stubs being submitted to the typeshed

I'll amend this a little. As another maintainer, I take objection since we were given no chance to check the quality of the stubs (and there are two different implementations?). Our API is complex enough (although seemingly simple) that I would want to be very certain the stubs are correct, not just close enough.

@o11c
Copy link
Contributor

o11c commented Nov 17, 2015

Now that six exists and typing is more complete, it should be a bit easier to write stubs in 2and3.

@sigmavirus24
Copy link

@o11c I think you're commenting on the wrong issue

@JukkaL
Copy link
Contributor Author

JukkaL commented Nov 18, 2015

My take on what happened is that the stubs in typeshed were imported from the mypy repo, and we (the mypy developers) didn't check with the module owners, though perhaps we should have -- the PEP only talks about typeshed, not tool-specific repos. When the stub migration happened I guess everybody assumed that somebody else had checked if including the stubs is fine.

I think that most of the third party stubs don't have many (or any) annotations so they should be reasonably safe from having bad annotations, but they might still get out of date and be incompatible with future API changes.

I guess we could retroactively ask for permission, or delete third party stubs with an unconfirmed acceptance status, but I'd propose to wait a little and see if we can come up with a reasonable general policy first.

Here are some potential issues:

  • What if a project has 100 (or just many) maintainers, or if the maintainer is a company, or if there is do declared owner? It seems totally impractical to get acceptance from every maintainer, or even a majority of them. I suggest that it's enough that we get permission from the owner or a lead maintainer of a project if we can find one, or otherwise from any active maintainer based on recent commit history. If other maintainers complain about the inclusion after the stub has already been accepted by another owner we'll probably have to sort it out it on a case by case basis.
  • What about modules where the maintainer doesn't wish stubs to be included in typeshed, and they don't want to provide their own stubs? I guess individual tools are free to do whatever they please, including the creation of custom stubs. This could be considered against the spirit of PEP 484. (Tools could decide to not use these custom stubs by default and require something like --unofficial-stubs to be used to enable them, but I'm not sure if anybody would be any happier.) Also, tools could automatically generate stubs based on the module implementation on a best-effort basis and these could be even worse than the stubs that would be available in typeshed. However, at least these wouldn't get stale.
  • Should we try to make it easy for project owners to review or comment on stubs that target their modules, maybe even on stub changes before they are merged? This way it would be less likely that bad stubs would go in, but this would require active involvement from project owners.
  • Should we have an audit trail of project owner acceptance? At the minimum we could include for each module the name of the owner who ok'd the inclusion of the stub.

@Lukasa
Copy link

Lukasa commented Nov 18, 2015

My 2¢:

What if a project has 100 (or just many) maintainers, or if the maintainer is a company, or if there is do declared owner?

Generally speaking such a project will have a formal decision making process. I recommend using it in that case. These projects are actually easier to keep track of than smaller ones because their decision-making process is formal and likely to be public. Consider using mailing lists etc.

What about modules where the maintainer doesn't wish stubs to be included in typeshed, and they don't want to provide their own stubs?

What about them? This isn't really typeshed's concern. In this instance, while typeshed has the right to do whatever you please, there appears to be a lot of consensus that maintainers should retain some degree of control over type stubs if possible. That means if a maintainer doesn't want any type stubs, you shouldn't create them. If another tool decides to ignore the maintainer, that antisocial behaviour is on them. Don't have typeshed abuse the wishes of a maintainer just because other tools will too.

Should we try to make it easy for project owners to review or comment on stubs that target their modules, maybe even on stub changes before they are merged?

It seems like this should be part of getting acceptance for new stubs. Here's an example policy:

  1. Bug report is opened, requesting that stubs be added for a project.
  2. You contact the project owners asking if they're ok with this. Your message should point them to PEP 484, to typeshed, and to the bug that was opened in step 1. You should tell them that if they say yes, they should follow the bug opened in step 1 to see how the work progresses. If this communication is public (e.g mailing list), link to that communication from the bug.
  3. Just before the work is merged, you inform the maintainers and give them a window of time to comment (or ask for more time to review): for example, a week.
  4. Merge

This is relatively slow, which is a downside, but it ensures good communication throughout and a fairly documentable process.

Should we have an audit trail of project owner acceptance?

You should really have something. How formal it is is a decision of typeshed, but at the very least it would be good to be able to open up a project stub and see who OKd the creation of it.

@sigmavirus24
Copy link

It may also be worth keeping documentation around who that stub's author was, if they plan to maintain it, and who on the project itself serves as a point of contact for OK'ing updates to the stubs. This ideally makes the process for reviewing PRs with updates to those stubs easier.

@jstasiak
Copy link
Contributor

Semi-related question - if a third party package comes with stubs (or type hints in the actual code) does it eliminate the need to maintain a separate set of stubs in typeshed? (Assuming mypy is executed in an environment that has access to the installed stubs).

@gvanrossum
Copy link
Member

@jstasiak It would be hard to maintain synchrony between the stubs in the release and the stubs in typeshed, so I think it would be better in that case to just use the stubs or annotations from the release. That's what MYPYPATH is for; and the --incremental flag should make it faster (it would avoid checking the annotated released code over and over). But the package owner has the last word (if they care -- most of them so far have just said "fine, do what you think is right").

It looks like Tornado might be adding annotations to a future release. I'm not sure what that means for the tornado stubs in typeshed, but surely pointing MYPYPATH at the tornado tree should force mypy to use the annotations from the tornado tree rather than from typeshed. The typeshed annotations might still be useful for folks stuck on an older tornado release; if the Tornado project asks us to remove them, I will do so, but I might first make a copy for my own use. :-)

@jstasiak
Copy link
Contributor

Cool, that's exactly what I thought. I'm asking because I'm interested in adding stubs/type hints to some libraries I'm responsible for to help move the static typing situation forward. :)

@JukkaL
Copy link
Contributor Author

JukkaL commented Apr 15, 2016

It may still make sense to have stubs in typeshed even if the release is annotated if the module is both popular and has a very stable (and thus likely small) interface.

@gvanrossum
Copy link
Member

I am closing this. We now have a documented process (https://github.com/python/typeshed#third_party) and we are following it. We accept proof of consent in the form of a response in the issue tracker for the 3rd party package.

cocoatomo pushed a commit to cocoatomo/typeshed that referenced this issue Sep 8, 2018
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

6 participants