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

Remove unused whitelist entries #4206

Closed
wants to merge 1 commit into from

Conversation

srittau
Copy link
Collaborator

@srittau srittau commented Jun 8, 2020

No description provided.

@hauntsaninja
Copy link
Collaborator

Well, it's annoying that this didn't pan out nicely.

The typing.IO.closed one is probably because Travis is using an older patch release from Github Actions; for once the issue stubtest found was on CPython's side and the fix got backported. (Note one fix here is to pin the patch version; I'd rather not do that since our policy is to support the latest patch version and we aren't going to be good at updating our CI for each patch release)

The tkinter ones are also "real". Those genuinely don't exist (were removed in 3.6). I think it's possible to wind up with Python builds without _tkinter, in which case the whole module fails to import (so dependent whitelist entries wouldn't surface errors). Haven't checked, but seems possible that Github Actions' build doesn't have _tkinter.

Not fully sure what's going on with os. docs.python.org says:

The MFD_HUGE* flags are only available since Linux 4.14.

So maybe Travis is using an old kernel?

The site one is super weird. Maybe pypa/virtualenv#737

We can "fix" any thorns using the (pattern)? trick that allows whitelist entries to go unused.

@srittau
Copy link
Collaborator Author

srittau commented Jun 8, 2020

Maybe we could just move all stubtests to GitHub? Then the Python version should be identical. Things should still get reported correctly.

@hauntsaninja
Copy link
Collaborator

hauntsaninja commented Jun 8, 2020

Sure. Seems like the only downside is stubtest wouldn't check anything in CI for changes dependent on _tkinter, but that seems fine. tk stubs are some of the worst anyway...

@hauntsaninja
Copy link
Collaborator

It's also weird that Github Actions has issues with site for the Python versions earlier than 3.8...
Anyway, in the meantime, I opened #4217 with the whitelist entry removals that should pass existing CI.

@srittau
Copy link
Collaborator Author

srittau commented Jun 10, 2020

Superseded by #4217.

@srittau srittau closed this Jun 10, 2020
srittau added a commit to srittau/typeshed that referenced this pull request Jun 11, 2020
This ensures that the Python version used matches the one used in the
scheduled extraneous stubtest whitelist check.

See also PR python#4206.
hauntsaninja pushed a commit that referenced this pull request Jun 11, 2020
This ensures that the Python version used matches the one used in the
scheduled extraneous stubtest whitelist check.

See also PR #4206.
vishalkuo pushed a commit to vishalkuo/typeshed that referenced this pull request Jun 26, 2020
This ensures that the Python version used matches the one used in the
scheduled extraneous stubtest whitelist check.

See also PR python#4206.
@srittau srittau deleted the unused-whitelist branch July 30, 2020 16:48
@Akuli
Copy link
Collaborator

Akuli commented Jan 11, 2021

Seems like the only downside is stubtest wouldn't check anything in CI for changes dependent on _tkinter

How was this problem solved when the CI moved to github actions?

It's possible to run Python with _tkinter on github actions. I use xvfb-action for that, like this:

https://github.com/Akuli/porcupine/blob/440018f19409e99b22b70893f51f357611122f04/.github/workflows/main.yml#L34-L38

@hauntsaninja
Copy link
Collaborator

hauntsaninja commented Jan 11, 2021

I'm sorry, I don't remember the specifics about what I was talking about here. (The "real" tkinter issues I mention were fixed in #4207)

Haven't checked, but seems possible that Github Actions' build doesn't have _tkinter.

It does seem to me that the Python build stubtest uses in Github Actions has _tkinter — are you seeing false negatives (or other issues) in CI?

Anyway, very happy that tk stubs are some of the worst anyway is no longer true. Thank you so much for that!

@Akuli
Copy link
Collaborator

Akuli commented Jan 12, 2021

Now that I think about it, stubtest has correctly pointed out many tkinter problems, so seems like it can at least import _tkinter. I think instantiating tkinter classes would fail though, because it's not ran inside xvfb.

Stubtest doesn't point out issues with attributes coming from __getattr__, and I think that's because it doesn't actually instantiate the classes that it imports. Most tkinter classes can be instantiated with no arguments, but again, that requires xvfb or a system with a display. But there are not many __getattr__s in tkinter.

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

Successfully merging this pull request may close these issues.

3 participants