Skip to content

Move builtin classmethod_descriptor to a different test #486

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 4 commits into from
Jan 17, 2023

Conversation

danigm
Copy link
Contributor

@danigm danigm commented Oct 28, 2022

This patch mvoes the builtin classmethod descriptor test to a different one and mark it to skip for python >= 3.10.8. The classmethod descriptor serializarion was removed from python in these releases: https://docs.python.org/3.10/whatsnew/changelog.html#id3

More information in the github issue:
python/cpython#95196

Fix #485

This patch mvoes the builtin classmethod descriptor test to a different
one and mark it to skip for python >= 3.10.8. The classmethod descriptor
serializarion was removed from python in these releases:
https://docs.python.org/3.10/whatsnew/changelog.html#id3

More information in the github issue:
python/cpython#95196

Fix cloudpipe#485
@codecov
Copy link

codecov bot commented Oct 28, 2022

Codecov Report

Base: 92.47% // Head: 83.12% // Decreases project coverage by -9.35% ⚠️

Coverage data is based on head (9b33280) compared to base (40aa846).
Patch has no changes to coverable lines.

❗ Current head 9b33280 differs from pull request most recent head 647b614. Consider uploading reports for the commit 647b614 to get more accurate results

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #486      +/-   ##
==========================================
- Coverage   92.47%   83.12%   -9.36%     
==========================================
  Files           4        3       -1     
  Lines         718      711       -7     
  Branches      159      153       -6     
==========================================
- Hits          664      591      -73     
- Misses         33       89      +56     
- Partials       21       31      +10     
Impacted Files Coverage Δ
cloudpickle/compat.py 60.00% <0.00%> (-40.00%) ⬇️
cloudpickle/cloudpickle_fast.py 80.73% <0.00%> (-15.91%) ⬇️
cloudpickle/cloudpickle.py 85.82% <0.00%> (-2.77%) ⬇️
cloudpickle/__init__.py

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

Copy link
Contributor

@ogrisel ogrisel left a comment

Choose a reason for hiding this comment

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

LGTM. We can later investigate we want to attempt classmethod descriptors in cloudpickle (for interactively defined classes) in a follow-up PR if someone has a need for it.

@ogrisel
Copy link
Contributor

ogrisel commented Oct 28, 2022

There are unrelated failures with the pytest module...

Copy link
Member

@pierreglaser pierreglaser left a comment

Choose a reason for hiding this comment

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

Thanks for the PR. Few suggestions, see review.

@pierreglaser
Copy link
Member

pierreglaser commented Nov 9, 2022

Thanks @danigm, waiting for CI and merging.

@ogrisel ogrisel mentioned this pull request Jan 17, 2023
@ogrisel
Copy link
Contributor

ogrisel commented Jan 17, 2023

Let's merge and fix the CI in #494.

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.

Tests fails on Python 3.10.8
3 participants