Update PyO3 and Rust Numpy to 0.18.0 #787
Conversation
The latest releases of PyO3 and rust numpy were just released. The changelogs for both can be found at: https://pyo3.rs/v0.18.0/changelog and https://github.com/PyO3/rust-numpy/blob/main/CHANGELOG.md Since the two releases are tightly coupled this commit opts to update them together instead of the normal dependabot workflow. Similarly some of our usage (mainly around signatures and argument ordering) for building a python interface using PyO3 has been deprecated. That usage is also updated in this PR to avoid compilation warnings, which will be treated as errors by clippy in CI.
Pull Request Test Coverage Report for Build 4056352977
💛 - Coveralls |
Fwiw, almost all of the changes here are just using the new |
| visitor: Option<PyBfsVisitor>, | ||
| ) -> PyResult<()> { | ||
| if visitor.is_none() { | ||
| return Err(PyTypeError::new_err("Missing required argument visitor")); | ||
| } |
There was a problem hiding this comment.
I think this is the only problematic part of the PR... Why can't digraph_bfs_search accept PyBfsVisitor? I think PyO3 should check that the argument is valid, not us
There was a problem hiding this comment.
Its the new rules about argument ordering, we're no longer allowed to have a required argument without a default defined after one an optional one that has a default. Basically it's trying to block a python signature like:
digraph_bfs_search(graph, source=None, visitor)which generally wouldn't be valid in python and is effectively what the previous signature for this function was.
There was a problem hiding this comment.
Basically we had two options to fix this, either give visitor a default option or move visitor before source. I opted to give it a default here because moving visitor would be a breaking change because it would break existing callers of this function using positional arguments
There was a problem hiding this comment.
Well, I guess we’ll have to deal with it
The latest releases of PyO3 and rust numpy were just released. The changelogs for both can be found at:
https://pyo3.rs/v0.18.0/changelog
and
https://github.com/PyO3/rust-numpy/blob/main/CHANGELOG.md
Since the two releases are tightly coupled this commit opts to update them together instead of the normal dependabot workflow. Similarly some of our usage (mainly around signatures and argument ordering) for building a python interface using PyO3 has been deprecated. That usage is also updated in this PR to avoid compilation warnings, which will be treated as errors by clippy in CI.