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

Add stubs for "netifaces" package #12854

Merged
merged 1 commit into from
Oct 21, 2024
Merged

Add stubs for "netifaces" package #12854

merged 1 commit into from
Oct 21, 2024

Conversation

Delgan
Copy link
Contributor

@Delgan Delgan commented Oct 18, 2024

Hi.

I generated stub files for the netifaces library.
Unfortunately, this library is no longer maintained (although still working well). For this reason, I would like to submit type hints to typeshed.

I compared to the types created initially by @jolaf in al45tair/netifaces#39 and it seems to match well.

@Delgan
Copy link
Contributor Author

Delgan commented Oct 18, 2024

PR updated with improved types contributed by @J-M0 here: al45tair/netifaces#77

This comment has been minimized.

1 similar comment

This comment has been minimized.

@jolaf
Copy link
Contributor

jolaf commented Oct 19, 2024

Thanks, good idea!!

Copy link
Collaborator

@srittau srittau left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks. Please be aware that we are currently adopting a new policy for unmaintained third-party stubs. It may be possible we will again remove these stubs if they start causing issues.

@@ -0,0 +1,74 @@
from typing import Dict, List, Tuple, Union, Literal
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

None of these – except Literal should be imported. Please use built-in types (dict, list, tuple) and new-style union syntax X | Y. (I thought our tests would complain about this, but they didn't ...)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

@@ -0,0 +1,74 @@
from typing import Dict, List, Tuple, Union, Literal

AF_12844: int
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These should all be marked Final[int].

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done as well.

@J-M0
Copy link
Contributor

J-M0 commented Oct 19, 2024

Hey all,

It so happens that I actually revisited these stubs I made a while ago and made some significant improvements, but never published them because I didn't think there was any interest. Is there any way we can integrate them into this?

@JelleZijlstra
Copy link
Member

Thanks @J-M0! There's a couple of approaches:

  • We could merge this PR, then you could make another PR with your improvements.
  • You could make a PR to @Delgan's fork with your changes, to get them merged into this PR branch.
  • You could open your own PR against typeshed and replace this one.

If the changes you're making are mostly about expanding the coverage (e.g., adding more types), then I'd advocate for the first one; it's good to start with simpler stubs and make them more comprehensive over time. If you're fixing incorrect types, then it's better to get them right the first time.

@J-M0
Copy link
Contributor

J-M0 commented Oct 19, 2024

@JelleZijlstra, the work I've done is mostly about making the return type for the gateways() function more precise. I also today incorporated @srittau's suggestion to mark constants as Final.

I've pushed my changes here so you can see them: https://github.com/J-M0/netifaces/blob/5dc562a0083b2a24f4fba33257a9b1aa654a3782/netifaces-stubs/__init__.pyi. Let me know how you think we should proceed.

Copy link
Contributor

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

@Delgan
Copy link
Contributor Author

Delgan commented Oct 19, 2024

Hey @J-M0, thanks for chiming in with additional improvements!

I updated my PR to use PEP 585 and add Final as requested by @srittau.
Your implementation of _Gateways looks very interesting. I was wondering how to encode the invariant of the "default" key, but I didn't think the type system could support it.

@Delgan Delgan requested a review from srittau October 19, 2024 17:18
@srittau srittau changed the title Add stubs for "netiface" package Add stubs for "netifaces" package Oct 21, 2024
@srittau srittau merged commit 0d7c6da into python:main Oct 21, 2024
47 checks passed
@jolaf
Copy link
Contributor

jolaf commented Oct 21, 2024

@srittau How long would it take for the changes to become available as types-netifaces in pip?

@srittau
Copy link
Collaborator

srittau commented Oct 21, 2024

Packages are built nightly (UTC time).

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.

5 participants