move the check bs if not None

[olegasics]

moved the check parameter to the top of the isnstance check, because if bs = None then an exception is thrown

[the-knights-who-say-ni]

Hello, and thanks for your contribution!

I'm a bot set up to make sure that the project can legally accept this contribution by verifying everyone involved has signed the PSF contributor agreement (CLA).

Recognized GitHub username

We couldn't find a bugs.python.org (b.p.o) account corresponding to the following GitHub usernames:

@olegasics

This might be simply due to a missing "GitHub Name" entry in one's b.p.o account settings. This is necessary for legal reasons before we can look at this contribution. Please follow the steps outlined in the CPython devguide to rectify this issue.

You can check yourself to see if the CLA has been received.

Thanks again for the contribution, we look forward to reviewing it!

[iritkatriel]

Thank you for your contribution.

This changes behavior, and if it didn't break any of the unit tests it means this case is not covered by tests and you need to fix that in this PR.

You also need to create an issue on bugs.python.org to describe this change and confirm that this is a bug and not a feature.

[iritkatriel]

Ah, it's not redundant:

isinstance(None, (bytes, bytearray))
False

so before the change None gave TypeError and after the change it returns ''.

[nanjekyejoannah]

I think thats what the logic should be, None == not bs hence '', Type Error is meant for the instance check, no? I will leave to you.

[iritkatriel]

I don't follow. The change is this:

def old():
    bs = None
    if not isinstance(bs, (bytes, bytearray)):
        raise TypeError("quote_from_bytes() expected bytes")
    if not bs:
        return ''

def new():
    bs = None
    if not bs:
        return ''
    if not isinstance(bs, (bytes, bytearray)):
        raise TypeError("quote_from_bytes() expected bytes")

print('--------- new:')
print(new() == '')
print('--------- old:')
print(old() == '')

output:

--------- new:
True
--------- old:
Traceback (most recent call last):
  File "C:\Users\User\src\cpython-current\tmp.py", line 20, in <module>
    print(old() == '')
  File "C:\Users\User\src\cpython-current\tmp.py", line 6, in old
    raise TypeError("quote_from_bytes() expected bytes")
TypeError: quote_from_bytes() expected bytes

[nanjekyejoannah]

hmm, I would see how returning TypeError for None would have been a feature and intentional hence this PR maybe actually wrong in that case.

[iritkatriel]

• bug tracker ticket
• unit test

[bedevere-bot]

A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated.

Once you have made the requested changes, please leave a comment on this pull request containing the phrase I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.

[iritkatriel]

Closed as per discussion on the pbo issue.