-
Notifications
You must be signed in to change notification settings - Fork 854
Add PyRange
wrapper
#5117
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
Add PyRange
wrapper
#5117
Conversation
16fc2e4
to
3545ea6
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm, I'm not super sure how I feel about these. There is a similarity to both range
and slice
here, but having conversions mixed like this and turning some into range
and others into slice
feels like a footgun to me. And from the Rust side I find Range
usually quite awkward to work with due to the limits their Iterator
implementation imposes. So I'm also not sure if there is great value in us providing these. (maybe new_range_api
could make a better interface in the future?)
Personally I think I'm leaning towards not having this (or dropping the conversions, I'd be fine with adding the type itself), but I'm keen to hear what others think about this.
Yes, The similar but so different |
PyRange
wrapper with conversions to/from std::ops::Range
PyRange
wrapper
impl<'py> TryFrom<Bound<'py, PyRange>> for Bound<'py, PySlice> { | ||
type Error = PyErr; | ||
|
||
fn try_from(range: Bound<'py, PyRange>) -> Result<Self, Self::Error> { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess we should add a test for this conversion
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry I missed this, will add it in a followup PR.
No description provided.