-
-
Notifications
You must be signed in to change notification settings - Fork 2.6k
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
Issue 7126 - "saferepr" to avoid BytesWarning when using --setup-show #7205
Conversation
bytes parametrize parameters cause error when --setup-show is used and Python is called with -bb flag
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for tackling this @lancelote. I left some comments.
GitLab is down so linting can't succeed, unfortunately. Changes after the review:
@bluetech Thank you for the valuable feedback 🙇🏻♂️ Please feel free to point me to any other improvements I can add here. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, LGTM, just a couple nits.
I'll leave this open for a few days to let other chime in if they have an opinion about the change to always use repr
(I think it makes sense and even in the str case makes it clearer), then I'll merge.
Co-authored-by: Ran Benita <ran@unusedvar.com>
Co-authored-by: Ran Benita <ran@unusedvar.com>
There was a problem hiding this 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 - let's merge!
Thanks, @lancelote 😁
Thanks everyone for the fix! 👍 |
Fix for #7126
I added a special case for bytes only as
saferepr
adds quotes to pure string literals. A bunch of tests depend on the original behavior, so I was not comfortable changing it globally. Scrappingsaferepr
output to match the original behavior doesn't sound as a trouble-proof solution too. Please correct me if it was a wrong idea.Thank you for your time reviewing the PR 🙏