Skip to content

revise PySuper::getattro same as CPython super_getattro#3058

Merged
youknowone merged 1 commit into
RustPython:mainfrom
youknowone:super-getattro
Sep 18, 2021
Merged

revise PySuper::getattro same as CPython super_getattro#3058
youknowone merged 1 commit into
RustPython:mainfrom
youknowone:super-getattro

Conversation

@youknowone

Copy link
Copy Markdown
Member

possibly related to #3057 but didn't solve the problem itself

@youknowone youknowone changed the title revise PySuper::getattro as CPython super_getattro revise PySuper::getattro same as CPython super_getattro Sep 14, 2021
@youknowone

Copy link
Copy Markdown
Member Author

@eldpswp99 could you review this PR? I think you also reviewed cpython super_getattro

@eldpswp99 eldpswp99 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.

LGTM! I think this revision follows CPython more than before.

@fanninpm

fanninpm commented Sep 15, 2021

Copy link
Copy Markdown
Contributor

Does this affect the results of test_super.py?

@youknowone

Copy link
Copy Markdown
Member Author

rebased it onto test_super.py but nothing changed.

@DimitrisJim DimitrisJim left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

making the names follow the ones used in CPython made this easy to review. p.s the soundforge bug report (SF ID #743627) is actually a dead-end, probably a relic left over in the CPython code.

@youknowone

Copy link
Copy Markdown
Member Author

I replaced it to alive link

@youknowone youknowone merged commit 8077e98 into RustPython:main Sep 18, 2021
@youknowone youknowone deleted the super-getattro branch September 18, 2021 05:12
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