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

Added optional encoding to dump and load functions #77

Merged
merged 1 commit into from
Jan 2, 2024

Conversation

Backist
Copy link
Contributor

@Backist Backist commented Dec 29, 2023

No description provided.

@Backist
Copy link
Contributor Author

Backist commented Dec 29, 2023

As i said in the issue, it is a minimal change but I think it is useful especially when you need to use a different encoding than the one your system uses, for example utf-8 instead of cp1292 which is used when the encoding parameter is not specified in open().

@bobfang1992 bobfang1992 merged commit 85a132e into bobfang1992:master Jan 2, 2024
4 of 7 checks passed
@bobfang1992
Copy link
Owner

Thanks, merged.

@bobfang1992
Copy link
Owner

@Backist it seems the ut are faling after I merge this, I can take a look at this later but if you have time please do not hesitate to have a look as well. Thanks!

Backist added a commit to Backist/pytomlpp that referenced this pull request Jan 2, 2024
@Backist
Copy link
Contributor Author

Backist commented Jan 2, 2024

The wheel error is essentially fixed, I just made a syntax error. I checked with the tests that the binary modes did not take the enconding argument as None, since the binary opening mode does not take an enconding.
I will make a new pull request with the hotfix.
Ignore that revert commit, I just created it by mistake

@Backist
Copy link
Contributor Author

Backist commented Jan 2, 2024

I'm a bit busy at the moment, let me have a look at it in depth as soon as I can verify with the tests before I do the final PR.

@bobfang1992
Copy link
Owner

@Backist sure thanks!

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.

2 participants