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.

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

binary mode doesn't take an encoding argument #664

Closed
yaythomas opened this issue Feb 3, 2022 · 5 comments
Closed

binary mode doesn't take an encoding argument #664

yaythomas opened this issue Feb 3, 2022 · 5 comments
Labels

Comments

@yaythomas
Copy link
Contributor

yaythomas commented Feb 3, 2022

The actual open() function does not allow you to pass an encoding when in binary mode:

>>> open('filename', 'wb', encoding='utf-8')
ValueError: binary mode doesn't take an encoding argument
>>> open('arb', 'b', encoding='utf-8')
ValueError: binary mode doesn't take an encoding argument

Assuming this is not an intentional divergence from open() behaviour, I think the fix would be to insert this:

if binary and encoding:
    raise ValueError("binary mode doesn't take an encoding argument")

just after here:
https://github.com/jmcgeheeiv/pyfakefs/blob/4dc8615957dadfc64d6a8f864ee93c7c65da38b5/pyfakefs/fake_filesystem.py#L5574

Before I prepare a PR, I thought I'd ask whether you think this is suitable first?

P.S this is an AMAZING project - it's my favourite new thing in my tool-belt! Thank you so much for all the work and care that went into this complex wonderful beast!

@mrbean-bremen
Copy link
Member

mrbean-bremen commented Feb 3, 2022

Thanks :) And good catch - no, this wasn't meant to diverge, we just overlooked it.
You mean if its a suitable first PR? Sure, go ahead!

@yaythomas
Copy link
Contributor Author

By suitable I meant more whether my suggested simplistic fix was comically naive and unaware of other complexities :-)

PR incoming. . .

@mrbean-bremen
Copy link
Member

The test coverage is pretty ok here, so it is not very likely that you will break something if the tests pass. And I guess you will add a test of your own for the change...

yaythomas added a commit to yaythomas/pyfakefs that referenced this issue Feb 3, 2022
                                                                                                                            On open() binary mode should not allow an encoding argument.
                                                                                                                            If encoding is passed with binary mode, raise:
                                                                                                                            ValueError(binary mode doesn\'t take an encoding argument)

                                                                                                                            Closes pytest-dev#664.
yaythomas added a commit to yaythomas/pyfakefs that referenced this issue Feb 3, 2022
                                                                                                                            On open() binary mode should not allow an encoding argument.
                                                                                                                            If encoding is passed with binary mode, raise:
                                                                                                                            ValueError(binary mode doesn\'t take an encoding argument)

                                                                                                                            Closes pytest-dev#664.
github-actions bot pushed a commit that referenced this issue Feb 3, 2022
…oding argument. If encoding is passed with binary mode, raise: ValueError(binary mode doesn\'t take an encoding argument) Closes #664.
@mrbean-bremen
Copy link
Member

Thank you, nice work!

@yaythomas
Copy link
Contributor Author

Pleasure was mine! Thanks for a smooth contribution experience!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants