add PyInt constructor for all supported number types (i32, u32, i64, u64, isize, usize)#4984
Conversation
…u64, isize, usize, f64) Solves a part of PyO3#2221
0744ce0 to
be85c1c
Compare
Icxolu
left a comment
There was a problem hiding this comment.
Thanks for working on this! In my opinion we should not introduce a new trait for this. This basically reimplements a subset of IntoPyObject. If we want to provide a generic constructor here, I think we should reuse the existing implementations and be generic over T: IntoPyObject<Target = PyInt, Error = Infallible>. I think that would cover all the (un)signed integer conversion. It would not cover f64 but I don't think that's a huge deal, it can easily be cast in Rust first and it is probably better to have this conversion explicit anyway.
|
Oh so borderline everything already exists through I presume the Netlify tests are failing because of things beyond my control? |
Icxolu
left a comment
There was a problem hiding this comment.
Thank you! Two small style suggestions otherwise LGTM 🚀
|
I have some ideas how to make the cache situation better so that the |
Solves a part of #2221.
I deliberately did not implement support for
PyLong_FromStringbecause I don't feel familiar enough with the codebase to do this in my first PR.I am aware that my
ToPyInttrait will not work withPyLong_FromString, becausePyLong_FromStringtakes more parameters than just a single value. I acknowledge that changing the trait to also accommodatePyLong_FromStringwill be a breaking change, but one could also add a dedicated constructor for that instead of squeezing it intonew.