Skip to content

Enable some metaclass tests#1179

Merged
slozier merged 2 commits intoIronLanguages:masterfrom
BCSharp:meta_tests
Apr 20, 2021
Merged

Enable some metaclass tests#1179
slozier merged 2 commits intoIronLanguages:masterfrom
BCSharp:meta_tests

Conversation

@BCSharp
Copy link
Copy Markdown
Member

@BCSharp BCSharp commented Apr 18, 2021

No description provided.

Copy link
Copy Markdown
Contributor

@slozier slozier left a comment

Choose a reason for hiding this comment

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

Looks good to me!

Comment thread Tests/test_metaclass.py Outdated

try_metaclass(type)
#try_metaclass(type(Old)) # bug 364938
try_metaclass(type(Old))
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.

We could probably just remove any Old related code since it's for old-style classes which no longer exist.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

There is a different code path in a few places between no base classes specified vs. explicit object as a base. The code should compensate for this but I would keep the tests. Admittedly, the names Old/New are misnomers, any ideas for better names?

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.

Can't think of good names, maybe something with ExplicitBase for the (object) version? We're going to need good comments for this otherwise I may forget about the different code paths and end up getting rid of it if I ever revisit.

This particular line though is identical to the line above since type(Old) is type.

@slozier slozier merged commit 9715cbc into IronLanguages:master Apr 20, 2021
@slozier
Copy link
Copy Markdown
Contributor

slozier commented Apr 20, 2021

Thanks!

@BCSharp BCSharp deleted the meta_tests branch April 20, 2021 15:49
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.

2 participants