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

Quality: Fixed the TypeError in autoformat #336

Merged
merged 3 commits into from Apr 9, 2019

Conversation

@bksahu
Copy link
Member

commented Apr 5, 2019

fixed #334

After rebasing I was getting another TypeError i.e. TypeError: write() argument must be str, not bytes

Thank your for contributing to Nuitka!

!! Please check that you select the develop branch (details see below link) !!

Before submitting a PR, please review the guidelines:
https://github.com/kayhayen/Nuitka/blob/master/CONTRIBUTING.md

What does this PR do?

Why was it initiated? Any relevant Issues?

PR Checklist

  • Correct base branch selected? develop for new features and bug fixes too. If it's
    part of a hotfix, it will be moved to master during it.
  • All tests still pass. Check the developer manual about Running the Tests. There
    are Travis tests that cover the most important things however, and you are
    welcome to rely on those, but they might not cover enough.
  • Ideally new features or fixed regressions ought to be covered via new tests.
  • Ideally new or changed features have documentation updates.
@kayhayen

This comment has been minimized.

Copy link
Member

commented Apr 5, 2019

This was during a commit, right? Because I think the code is supposed to be a str, but getFileHashContent produces bytes I think. Just do the if str is not bytes: ...decode("utf8") dance there, because that is wrong. Right now getFileContent gives str, and getHashContent gives bytes, not a good idea.

That said, for non-utf8 code, we are going to have a hard time this way... and maybe, the old_code needs to be bytes, and when we add a mode to getFileContent, allowing us to say rb there, and then make sure we work on bytes all the time.

@bksahu

This comment has been minimized.

Copy link
Member Author

commented Apr 5, 2019

Yes, it was during a commit. I will try to submit a patch tomorrow morning.

@kayhayen

This comment has been minimized.

Copy link
Member

commented Apr 6, 2019

@bksahu You are compensating in your change for the difference bytes from git, unicode from filesystem, when you write the file. But I think that "old_code" is also later compared, and again that issue will be there.

        changed = False
        if old_code != getFileContents(tmp_filename):
            my_print("Updated.")

So I am thinking, instead of converting back and forth, make sure to write the file with wb and teach getFileContents to allow passing the mode, so you can make this and above getFileContents(tmp_filename, "rb").

Always make sure to solve your data problems at the inception. So reading the files as "unicode" has no value in here, as we will have to know the encoding, and limit ourselves, so e.g. no test can have latin1 anymore. To copy, compare, we can work with bytes. Then black, isort, and friends can deal with it when reading from the file.

Do you get me there?

@bksahu

This comment has been minimized.

Copy link
Member Author

commented Apr 6, 2019

Yes. We want old_code to be in a single format i.e Bytes and not Unicode throughout the whole process.

@kayhayen kayhayen added the gsoc2019 label Apr 8, 2019

@kayhayen kayhayen self-assigned this Apr 8, 2019

@kayhayen kayhayen merged commit 5d4594b into Nuitka:develop Apr 9, 2019

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
@kayhayen

This comment has been minimized.

Copy link
Member

commented Apr 9, 2019

Thanks, good job.

@bksahu

This comment has been minimized.

Copy link
Member Author

commented Apr 9, 2019

@kayhayen Thank you for creating git hooks. It will save a lot of time. ☺️

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.