Skip to content

Py value into ref #678

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 1 commit into from
Mar 14, 2019
Merged

Py value into ref #678

merged 1 commit into from
Mar 14, 2019

Conversation

cthulahoops
Copy link
Collaborator

@cthulahoops cthulahoops commented Mar 14, 2019

As discussed in #674.

My plan is:

  • eliminate PyRef::new*
  • move all/most calls to PyObject::new(...) to this interface.
  • use this narrow interface to solve the which objects get attributes problem.

Pushing this push first to make sure we agree on the interface.

@codecov-io
Copy link

Codecov Report

Merging #678 into master will decrease coverage by 0.03%.
The diff coverage is 0%.

Impacted file tree graph

@@            Coverage Diff            @@
##           master    #678      +/-   ##
=========================================
- Coverage   40.33%   40.3%   -0.04%     
=========================================
  Files          79      79              
  Lines       17488   17503      +15     
  Branches     4470    4474       +4     
=========================================
  Hits         7054    7054              
- Misses       8588    8603      +15     
  Partials     1846    1846
Impacted Files Coverage Δ
vm/src/pyobject.rs 57.34% <0%> (-0.87%) ⬇️

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 964a041...b204c2b. Read the comment docs.

Copy link
Contributor

@OddCoincidence OddCoincidence left a comment

Choose a reason for hiding this comment

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

I like it! This will also make calling python methods from rust more composable given the convention of accepting refs and returning by value, e.g.

let s: PyStringRef;
...
s.upper().into_ref().ends_with(<suffix>)

(I do wonder if the Sized bound will be an issue w/ the object representation changes I have brewing, but if so, it's easy enough to split this into another trait with a blanket impl on PyValue)

@cthulahoops
Copy link
Collaborator Author

Remember you need a context for into_ref.

@cthulahoops cthulahoops merged commit 529da3f into master Mar 14, 2019
@cthulahoops cthulahoops deleted the py_value_into_ref branch April 9, 2019 19:53
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