Skip to content

Conversation

@stratakis
Copy link

Currently the test_sane_inheritance fail for Python 3.6

This is due to the changes done at: http://bugs.python.org/issue23722

Also some more details can be found here: https://docs.python.org/3/reference/datamodel.html#creating-the-class-object

This pull request addresses this issue for Python 3.6.

@encukou encukou mentioned this pull request Jan 3, 2017
@stratakis
Copy link
Author

Also kudos to @encukou for his help with this PR

Copy link

@gustavlarson gustavlarson left a comment

Choose a reason for hiding this comment

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

This change is not compiling, but after fixing them the unit tests pass on 3.6.

conv_nmspc["__module__"] = nmspc["__module__"]
if qn is not None:
conv_nmspc["__qualname__"] = qn
if classcell is not None

Choose a reason for hiding this comment

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

missing : after None

qn = nmspc["__qualname__"]
nmspc["__qualname__"] = qn + "Base"

classcell = nmspc.pop('__classcell__', None)

Choose a reason for hiding this comment

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

Initialize classcell to None before line 107

@stratakis
Copy link
Author

Thanks for taking a look at it. Made the changes you requested.

Copy link

@gustavlarson gustavlarson left a comment

Choose a reason for hiding this comment

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

Nice!

conv_nmspc["__module__"] = nmspc["__module__"]
if qn is not None:
conv_nmspc["__qualname__"] = qn
if classcell is not None:
Copy link

Choose a reason for hiding this comment

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

nit: two spaces in is not here

Copy link

@ktdreyer ktdreyer left a comment

Choose a reason for hiding this comment

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

@ajtowns mind merging this?

Copy link

@StykMartin StykMartin left a comment

Choose a reason for hiding this comment

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

Any plans for merge? Or we can consider this project as completely dead.

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.

4 participants