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

Bump networkx to 3.2.1 #11336

Merged
merged 10 commits into from
Jan 31, 2024
Merged

Bump networkx to 3.2.1 #11336

merged 10 commits into from
Jan 31, 2024

Conversation

Avasam
Copy link
Collaborator

@Avasam Avasam commented Jan 29, 2024

Follow-up to #10544
Closes #11169

I hit a couple limitations along the way, I'll open and link relevant issues

I've added github comments to files that were more than just adding @_dispatch decorator to make them easier to find and review.

stubsabot and others added 5 commits December 16, 2023 00:11
Release: https://pypi.org/pypi/networkx/3.2.1
Homepage: https://networkx.org/
Repository: https://github.com/networkx/networkx

If stubtest fails for this PR:
- Leave this PR open (as a reminder, and to prevent stubsabot from opening another PR)
- Fix stubtest failures in another PR, then close this PR

Note that you will need to close and re-open the PR in order to trigger CI
# Correct usage
assert_type(some_method(0, ""), float)
# type system doesn't allow this yet (see comment in networkx/utils/backends.pyi)
# assert_type(some_method(0, "", backend=None), float)
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This would require some way of concatenating backend to _P.kwargs. Which to my understanding, isn't currently possible and is a rejected idea in https://peps.python.org/pep-0612/#concatenating-keyword-parameters

This comment has been minimized.

@overload
def cliques_containing_node(
G: Graph[_Node], nodes: _Node, cliques: Iterable[Container[_Node]] | None = None
) -> Generator[list[_Node], None, None]: ...
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

These methods have been removed

) -> OutMultiEdgeDataView[_Node, tuple[_Node, _Node, _U]]: ...
@overload # type: ignore[override] # Has an additional `keys` keyword argument
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Source has improved overridden methods arguments

@@ -87,14 +84,14 @@ def set_node_attributes(
values: SupportsItems[_Node, SupportsKeysAndGetItem[Incomplete, Incomplete] | Iterable[tuple[Incomplete, Incomplete]]],
name: None = None,
) -> None: ...
def get_node_attributes(G: Graph[_Node], name: str) -> dict[_Node, Incomplete]: ...
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

New arguments and less re-exports in this file

) -> DiGraph[_Node]: ...
@overload
def subgraph_view(
G: Graph[_Node], filter_node: Callable[[_Node], bool] = ..., filter_edge: Callable[[_Node, _Node], bool] = ...
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

simple kw-only change on this method

def node_link_data(
G,
attrs: Incomplete | None = None,
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Removed param

Comment on lines +29 to +59
def __new__(
cls,
func: Callable[_P, _R] | None = None,
*,
name: str | None = None,
graphs: str | None | Mapping[str, int] = "G",
edge_attrs: str | dict[str, Any] | None = None,
node_attrs: str | dict[str, Any] | None = None,
preserve_edge_attrs: bool = False,
preserve_node_attrs: bool = False,
preserve_graph_attrs: bool = False,
preserve_all_attrs: bool = False,
) -> Self: ...
@property
def __doc__(self): ...
@__doc__.setter
def __doc__(self, val) -> None: ...
@property
def __signature__(self): ...
# Type system limitations doesn't allow us to define this as it truly should.
# But specifying backend with backend_kwargs isn't a common usecase anyway
# and specifying backend as explicitely None is possible but not intended.
# If this ever changes, update stubs/networkx/@tests/test_cases/check_dispatch_decorator.py
@overload
def __call__(self, *args: _P.args, **kwargs: _P.kwargs) -> _R: ...
@overload
def __call__(self, *args: Any, backend: str, **backend_kwargs: Any) -> _R: ...
# @overload
# def __call__(self, *args: _P.args, backend: None = None, **kwargs: _P.kwargs) -> _R: ...
# @overload
# def __call__(self, *args: _P.args, backend: str, **kwargs: _P.kwargs, **backend_kwargs: Any) -> _R: ...
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I've only completed __new__ and __call__ here.
As far as I know, I'm limited by the type system here because I can't concatenate kwargs, Please let me know if there's a way to actually do it.

This comment has been minimized.

from networkx.linalg import *
from networkx.readwrite import *
from networkx.relabel import *
from networkx.utils.backends import _dispatch as _dispatch
Copy link
Collaborator Author

@Avasam Avasam Jan 29, 2024

Choose a reason for hiding this comment

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

New re-exports (but they're also private, could be omitted)

srittau
srittau previously approved these changes Jan 29, 2024
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.

Due to the size of the PR, I didn't review it in depth, but your comments make sense to me.

This comment has been minimized.

@srittau
Copy link
Collaborator

srittau commented Jan 30, 2024

Some genuine regression test errors.

@srittau srittau dismissed their stale review January 30, 2024 18:13

Regression test errors.

This comment has been minimized.

Copy link
Contributor

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

@srittau srittau merged commit faf9d77 into python:main Jan 31, 2024
45 checks passed
@Avasam Avasam deleted the Bump-networkx-to-3.2.1 branch January 31, 2024 07:17
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.

2 participants