Skip to content

Conversation

@T4rk1n
Copy link
Contributor

@T4rk1n T4rk1n commented Oct 29, 2025

Recreation of #3476 with further fixes

Fix #3139

@T4rk1n T4rk1n requested review from emilykl and ndrezn as code owners October 29, 2025 13:39
@T4rk1n
Copy link
Contributor Author

T4rk1n commented Nov 5, 2025

@KoolADE85 I had to make a bunch of changes to make the typing tests pass, can you take a final look?

Copy link

@camdecoster camdecoster left a comment

Choose a reason for hiding this comment

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

Looks good. I made a few comments, but nothing stopping merge.

Choose a reason for hiding this comment

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

Do you have a convention for using import typing vs. from typing import ...? If so, could you switch to that convention across all these type annotations?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No conventions, some types are better import from typing_extensions to ensure backward compatibility in which case I prefer the from typing import ... syntax to easily switch between those.

Comment on lines +575 to +578
self.callback_map: dict = {}
# same deps as a list to catch duplicate outputs, and to send to the front end
self._callback_list = []
self.callback_api_paths = {}
self._callback_list: list = []
self.callback_api_paths: dict = {}

Choose a reason for hiding this comment

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

Does dict assign the same type as Dict?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, we still support 3.8 but later versions of Python prefer using that with the list[int] syntax.

@T4rk1n T4rk1n merged commit b57e0f5 into dev Nov 6, 2025
28 of 30 checks passed
@T4rk1n T4rk1n deleted the fix-pkgutil branch November 6, 2025 19:32
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.

Fix pkgutil.find_loader deprecation warning

4 participants