search

PyO3 / rust-numpy

PyO3-based Rust bindings of the NumPy C-API
BSD 2-Clause "Simplified" License
1.11k stars 106 forks source link

Remove unneeded lifetime annotations #389

Closed Hofer-Julian closed 1 year ago

Hofer-Julian commented 1 year ago

It seems like none of these annotations are necessary. After removing them, everything still compiles and works as expected.

adamreichold commented 1 year ago

@Hofer-Julian The Clippy lint is unrelated, but could you fix it nevertheless by explicitly allowing the lint? (We want to test this case even if we do not need a Vec to store 1, 2 and 3.)

Hofer-Julian commented 1 year ago

Couldn't test it easily locally. Let's see if this does the trick.

adamreichold commented 1 year ago

Thanks!

mejrs commented 1 year ago

These lifetimes are on purpose, see also https://doc.rust-lang.org/rustc/lints/listing/allowed-by-default.html#elided-lifetimes-in-paths . Can this PR be reverted?

adamreichold commented 1 year ago

These lifetimes are on purpose, see also https://doc.rust-lang.org/rustc/lints/listing/allowed-by-default.html#elided-lifetimes-in-paths . Can this PR be reverted?

While I see their value as documentation I am also not particularly keen on having them (i.e. I did not enable the lint). Do you see any functional issues with removing them?

(Reverting is obviously technically possible, but I am just not invested one way or the other. If we do revert because we really want the lifetimes, we should probably enable the lint though.)

mejrs commented 1 year ago

I don't actually care that much because I don't feel that strongly about rust-numpy but I enabled that lint in pyo3 because I've seen many newbies struggle because of hidden lifetimes parameters. My preference is to enable that lint everywhere.