Skip to content

More post-Python 2 cleanups #345

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 2 commits into from
Feb 15, 2020
Merged

Conversation

ogrisel
Copy link
Contributor

@ogrisel ogrisel commented Feb 15, 2020

I found some dead-code that could be removed now that we are a Python 3.5+ only project.

The coverage report still has holes that should be plugged in later PRs.

@@ -786,51 +783,6 @@ def save_instancemethod(self, obj):

dispatch[types.MethodType] = save_instancemethod

def save_inst(self, obj):
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This as called in a Python 2 only branch of save_instancemethod.

@@ -580,6 +576,7 @@ def save_dynamic_class(self, obj):

# If type overrides __dict__ as a property, include it in the type
# kwargs. In Python 2, we can't set this attribute after construction.
# XXX: can this ever happen in Python 3? If so add a test.
__dict__ = clsdict.pop('__dict__', None)
if isinstance(__dict__, property):
type_kwargs['__dict__'] = __dict__
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not so sure about this: type_kwargs['__dict__'] = __dict__ is no longer covered by our tests.

@ogrisel ogrisel force-pushed the more-python-2-cleanups branch from 6778212 to f7922a4 Compare February 15, 2020 11:21
@ogrisel
Copy link
Contributor Author

ogrisel commented Feb 15, 2020

It's sad that we cannot get the codecov reports in PR because the CODECOV_TOKEN secret is not available on pull-requests.

This is probably a design limitation of codecov. I do not see any way around as long as use github actions and codecov does not provide native upload access checks for this CI.

@ogrisel ogrisel merged commit 0a907cc into cloudpipe:master Feb 15, 2020
@ogrisel ogrisel deleted the more-python-2-cleanups branch February 15, 2020 11:25
@ogrisel
Copy link
Contributor Author

ogrisel commented Feb 15, 2020

@pierreglaser I merged to get the codecov report updated on master. The remaining commented points can be addressed in later PRs.

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.

None yet

1 participant