Skip to content

Introduce Either extractor and convert range.__getitem__ #739

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

Merged
merged 5 commits into from
Mar 24, 2019

Conversation

OddCoincidence
Copy link
Contributor

@OddCoincidence OddCoincidence commented Mar 23, 2019

Note: I intentionally implemented TryFromObject only for an Either of two PyRefs rather than any two types than implement TryFromObject. This is because doing the latter would pointlessly allocate an error object whenever the argument was type B. By restricting this to PyRefs, we can just try to downcast, which is a lot cheaper.

Example of the error messages this returns:

>>>>> range(10).__getitem__('')
Traceback (most recent call last):
  File <stdin>, line 0, in <module>
TypeError: must be int or slice, not str

@codecov-io
Copy link

Codecov Report

Merging #739 into master will increase coverage by 0.08%.
The diff coverage is 15%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #739      +/-   ##
==========================================
+ Coverage   44.69%   44.78%   +0.08%     
==========================================
  Files          81       81              
  Lines       16390    16379      -11     
  Branches     4089     4093       +4     
==========================================
+ Hits         7326     7335       +9     
+ Misses       7275     7258      -17     
+ Partials     1789     1786       -3
Impacted Files Coverage Δ
vm/src/obj/objslice.rs 38.55% <0%> (-10.1%) ⬇️
vm/src/pyobject.rs 55.65% <37.5%> (-1.19%) ⬇️
vm/src/obj/objrange.rs 41.31% <7.14%> (+8.38%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a156010...84d47d2. Read the comment docs.

///
/// Note: The returned `Result` is _not_ a `PyResult`, even though the
/// types are compatible.
pub fn downcast<T: PyObjectPayload>(self: Rc<Self>) -> Result<PyRef<T>, PyObjectRef> {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought arbitrary self types hadn't landed yet?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It hasn't for user-defined types, but Box<Self> and Rc<Self> as self types are allowed.

@OddCoincidence OddCoincidence merged commit 5441f3f into master Mar 24, 2019
@OddCoincidence OddCoincidence deleted the joey/range-getitem-either branch April 9, 2019 20:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants