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

Merge openpyxl stubs from microsoft/python-type-stubs #9511

Draft
wants to merge 98 commits into
base: main
Choose a base branch
from

Conversation

Avasam
Copy link
Collaborator

@Avasam Avasam commented Jan 12, 2023

Bring openpyxl stubs at least up to par with https://github.com/microsoft/python-type-stubs/tree/main/openpyxl

Add class variable types and descriptors for affected classes.
For a bit of context: "descriptor" types are assigned to class variables. Those become property-like with getters that type-check on assignment.
There's also "Convertible" descriptors that do type coercion.

This is not meant to complete the stub, far from there. Only to make it good enough to upstream. And a few other things here and there.

There's overlap with #9487 #9764, I'd suggest finishing that PR first, then I'll double check any conflict that arise.
I'll look at primer results after that as well.

@github-actions

This comment has been minimized.

@AlexWaygood
Copy link
Member

(Adding DO-NOT-MERGE for now, as I agree we should resolve #9487 first before tackling this :)

@Avasam
Copy link
Collaborator Author

Avasam commented Jan 12, 2023

In the meantime, @AlexWaygood , can we consider allowing lxml-stubs in the stubuploader?

Also, is there a list somewhere of those "stubs taken out of typeshed because incomplete"? I feel it shouldn't be too hard to add 'em back with modern stubgen.

@AlexWaygood
Copy link
Member

In the meantime, @AlexWaygood , can we consider allowing lxml-stubs in the stubuploader?

SGTM but I'm not a stub-uploader maintainer :)

@AlexWaygood
Copy link
Member

AlexWaygood commented Jan 12, 2023

Also, is there a list somewhere of those "stubs taken out of typeshed because incomplete"? I feel it shouldn't be too hard to add 'em back with modern stubgen.

Not sure I understand, sorry -- what's this a reference to?

@Avasam
Copy link
Collaborator Author

Avasam commented Jan 12, 2023

Not sure I understand, sorry -- what's this a reference to?

https://pypi.org/project/lxml-stubs/

These type annotations were initially included in typeshed, but lxml's annotations are still incomplete and have therefore been extracted from typeshed to avoid unintentional false positive results.

I feel it's not the first time I see that. Or maybe it is and I just read it twice on the same page.

@AlexWaygood
Copy link
Member

You'll have to ask one of the other maintainers -- sounds like that all happened before I arrived on the typeshed scene!

@github-actions

This comment has been minimized.

@AlexWaygood
Copy link
Member

In the meantime, @AlexWaygood , can we consider allowing lxml-stubs in the stubuploader?

SGTM but I'm not a stub-uploader maintainer :)

On second thought -- with our current setup, I think you'll be unable to use lxml-stubs even if wwe add them to the stub-uploader allowlist :( The current rules on the stub-uploader are that non-types dependencies must be explicitly included in the "non-types allowlist", and also be specified as a requirement of the runtime package. But the runtime package doesn't declare lxml-stubs as a dependency, so the stub-uploader tests will fail even if we add lxml-stubs in the stub-uploader :/

This is an annoying situation but I'm not sure what the solution might be.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

Copy link
Contributor

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

@Avasam Avasam mentioned this pull request Mar 13, 2024
Copy link
Contributor

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

Copy link
Contributor

github-actions bot commented Apr 1, 2024

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

Copy link
Contributor

github-actions bot commented Apr 8, 2024

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

Copy link
Contributor

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

Copy link
Contributor

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

@JelleZijlstra
Copy link
Member

@Avasam this is now the oldest open PR and CI is green, but it's still in draft. Is it ready now? If not, what is the plan for this PR?

@Avasam
Copy link
Collaborator Author

Avasam commented Oct 2, 2024

I've been extracting changes by scope and concepts they relate to, which allowed me to validate the changes actually made sense and fix entire sections of openpyxl stubs at once. (this PR started with over 80 files changed)

The only thing left here would be a PR that covers adding annotations for openpyxl Cell Values and that should close this PR.

Copy link
Contributor

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

Copy link
Contributor

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

Copy link
Contributor

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

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

Successfully merging this pull request may close these issues.

6 participants