Skip to content

test properties on a new-style class. - #332

Merged
pierreglaser merged 2 commits into
cloudpipe:masterfrom
pierreglaser:fix-test-property-py27
Jan 29, 2020
Merged

test properties on a new-style class.#332
pierreglaser merged 2 commits into
cloudpipe:masterfrom
pierreglaser:fix-test-property-py27

Conversation

@pierreglaser

Copy link
Copy Markdown
Member

Follow up on #329
Properties only work on new-style classes, that's why test_property is failing on Python2 tests.

Properties only work on new-style classes
@ogrisel

Copy link
Copy Markdown
Contributor

Could you please add a comment to explain this?

@ogrisel

Copy link
Copy Markdown
Contributor

BTW we should consider making a new major cloudpickle release that stop supporting Python 2 in the coming months :)

@ogriselogrisel left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

+1 for merge once the comment is added.

@codecov

codecovBot commented Jan 29, 2020

Copy link
Copy Markdown

Codecov Report

Merging #332 into master will increase coverage by 12.44%.
The diff coverage is n/a.

Impacted file tree graph

@@ Coverage Diff @@## master #332 +/- ##
===========================================
+ Coverage 53.05% 65.49% +12.44% 
===========================================
Files 2 2 Lines 852 855 +3 Branches 176 177 +1 ===========================================
+ Hits 452 560 +108 + Misses 365 269 -96 + Partials 35 26 -9
Impacted FilesCoverage Δ
cloudpickle/cloudpickle.py89.6% <0%> (+16.93%)⬆️

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 6f4c560...ad28cf2. Read the comment docs.

@pierreglaser
pierreglaser merged commit 4e57a45 into cloudpipe:masterJan 29, 2020
@pierreglaser
pierreglaser deleted the fix-test-property-py27 branch January 29, 2020 20:14
Sign up for freeto 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.

2 participants

@pierreglaser@ogrisel