Skip to content

Unify property like objects #635

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 6 commits into from
Mar 10, 2019
Merged

Unify property like objects #635

merged 6 commits into from
Mar 10, 2019

Conversation

BenLewis-Seequent
Copy link

Followup to #619, migrates property to the newer style and unify property like objects by:

  • making data descriptors an actual property, removing the need for an extra class.
  • making any property created from rust, which only has a getter(any use of vm.new_property) a readonly property. This means they can be overridden by the instance dict, hopefully leading to more consistent behavior.
  • changes member descriptors to be properties

@codecov-io
Copy link

codecov-io commented Mar 9, 2019 •
edited
Loading

Codecov Report

Merging #635 into master will increase coverage by 0.08%.
The diff coverage is 61.64%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #635      +/-   ##
==========================================
+ Coverage   40.81%   40.89%   +0.08%     
==========================================
  Files          76       76              
  Lines       17345    17384      +39     
  Branches     4474     4479       +5     
==========================================
+ Hits         7079     7109      +30     
- Misses       8366     8384      +18     
+ Partials     1900     1891       -9
Impacted Files Coverage Δ
vm/src/obj/objfunction.rs 36.79% <100%> (-1.83%) ⬇️
vm/src/obj/objobject.rs 43.1% <42.85%> (+1.06%) ⬆️
vm/src/obj/objcode.rs 31.57% <50%> (-0.38%) ⬇️
vm/src/obj/objtype.rs 42.85% <50%> (+0.98%) ⬆️
vm/src/obj/objproperty.rs 58.49% <56.86%> (+17.31%) ⬆️
vm/src/pyobject.rs 59.58% <78.57%> (-1.35%) ⬇️
vm/src/vm.rs 54.46% <80%> (+0.01%) ⬆️
vm/src/function.rs 67.24% <84.61%> (+6.37%) ⬆️
... and 6 more

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 010969f...7d01323. Read the comment docs.

@BenLewis-Seequent BenLewis-Seequent mentioned this pull request Mar 9, 2019
Merged
windelbouwman
windelbouwman reviewed Mar 9, 2019
View reviewed changes
@windelbouwman
Copy link
Contributor

@cthulahoops could you have a look at this? It looks fine to me, but you are more into the descriptor things.

@cthulahoops cthulahoops merged commit 9e5b76c into RustPython:master Mar 10, 2019
@BenLewis-Seequent BenLewis-Seequent deleted the property_refactor branch March 29, 2019 06:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@windelbouwman windelbouwman windelbouwman left review comments

@cthulahoops cthulahoops cthulahoops approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@BenLewis-Seequent @codecov-io @windelbouwman @cthulahoops @skinnyBat