Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Fix #1285, text_signature and raw ident interaction #1286

Merged
merged 1 commit into from
Nov 20, 2020
Merged

Fix #1285, text_signature and raw ident interaction #1286

merged 1 commit into from
Nov 20, 2020

Conversation

roblabla
Copy link
Contributor

@roblabla roblabla commented Nov 19, 2020 •
edited
Loading

Fixes #1285

Fix ended up being simple: parse_text_signature_attrs took the raw name instead of the pythonized name. I added some simple tests to make sure we avoid regressions.

@roblabla roblabla force-pushed the fix-1285 branch 4 times, most recently from d25ba31 to 4b18c9a Compare November 19, 2020 21:43
davidhewitt
davidhewitt approved these changes Nov 19, 2020
View reviewed changes
Copy link
Member

@davidhewitt davidhewitt left a comment

Choose a reason for hiding this comment

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

👍 Thanks very much for fixing this, will merge shortly assuming CI passes!

@davidhewitt
Copy link
Member

Hmm looks like the test isn't passing? 🤔

@roblabla
Copy link
Contributor Author

I got no clue what's going on, the test passes when locally running cargo test test_raw_identifiers.

@davidhewitt
Copy link
Member

davidhewitt commented Nov 20, 2020 •
edited
Loading

Ahh I think __text_signature__ doesn't work with abi3 feature (until Python 3.10, where it might start working). (Opened #1288 to track that.)

I've pushed a tweak to your commit which just ignores the test in that situation.

kngwyu
kngwyu approved these changes Nov 20, 2020
View reviewed changes
Copy link
Member

@kngwyu kngwyu left a comment

Choose a reason for hiding this comment

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

Wow, what a simple fix ❤️

@kngwyu
Copy link
Member

kngwyu commented Nov 20, 2020

Thanks @roblabla and @davidhewitt

@kngwyu kngwyu merged commit 096b0a3 into PyO3:master Nov 20, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@davidhewitt davidhewitt davidhewitt approved these changes

@kngwyu kngwyu kngwyu approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

text_signature and raw identifiers don't interact well
3 participants
@roblabla @davidhewitt @kngwyu