RSDK-9590 - upgrade numpy #805
Conversation
stuqdog
requested review from
purplenicole730,
lia-viam,
njooma and
penguinland
| @@ -16,7 +16,7 @@ buf: clean | |||
| rm -rf src/viam/gen | |||
| chmod +x plugin/main.py | |||
| uv pip install protoletariat | |||
| uv pip install protobuf --upgrade | |||
| uv pip install protobuf==5.29.1 | |||
There was a problem hiding this comment.
@njooma this was the one part I wasn't sure on from your commit, I went with the hard locking to 5.29.1 but will change back if that's not right!
There was a problem hiding this comment.
👍 I like pinning the version used! That way, when a new employee joins the team in 6 months and gets protobuf 5.30.0, you don't need to track down bugs that happen on your machine but not on theirs, or vice versa.
Slight preference towards pinning the proletariat version (and all other dependencies), too, but that's perhaps off-topic for the current PR.
| @@ -16,7 +16,7 @@ buf: clean | |||
| rm -rf src/viam/gen | |||
| chmod +x plugin/main.py | |||
| uv pip install protoletariat | |||
| uv pip install protobuf --upgrade | |||
| uv pip install protobuf==5.29.1 | |||
There was a problem hiding this comment.
👍 I like pinning the version used! That way, when a new employee joins the team in 6 months and gets protobuf 5.30.0, you don't need to track down bugs that happen on your machine but not on theirs, or vice versa.
Slight preference towards pinning the proletariat version (and all other dependencies), too, but that's perhaps off-topic for the current PR.
Yeah I'm inclined to leave that for a future PR, though I do like the suggestion! |
pyproject.toml
Outdated
| "numpy>=1.26.2,<2; python_version>='3.9' and python_version <'3.13'", | ||
| "numpy>=2; python_version>='3.13'", |
There was a problem hiding this comment.
I don't think we need all of this anymore. I think we just need "numpy<1.25.0; python_version<'3.9'",, and then "numpy>=1.26.2; python_version>='3.9'",
There was a problem hiding this comment.
ooh yeah, good point! will update.
There was a problem hiding this comment.
As an aside, that does mean we'll need to update the version check logic in flat_tensors_to_ndrrays to check the numpy version rather than the python version.
No description provided.