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

Upgrade to all f-strings #1567

Closed
fubuloubu opened this issue Aug 8, 2019 · 3 comments · Fixed by #1614

Comments

@fubuloubu
Copy link
Member

@fubuloubu fubuloubu commented Aug 8, 2019

What's your issue about?

The code uses a mix of %s, .format(), and f"", which can be confusing which style we prefer.

How can it be fixed?

Change it all to one style, preferably f-strings.

Also, document this in the style guide

@wschwab

This comment has been minimized.

Copy link
Contributor

@wschwab wschwab commented Sep 11, 2019

I'd like to take this. 2 questions:

  1. When would you like to have this done by?
  2. Where is the style guide?

I just started poking around, and quickly ran into another issue:
I see that there are places where byte strings are formatted (see vyper/tests/types/test_bytes_literal.py on lines 48, 56, and 67 as of when I'm writing this), and there doesn't seem to be any way to make a 'byte-fstring' (in fact, even .format doesn't work, only % and some other hacks). What should the format be for byte strings that need formatting?

@fubuloubu

This comment has been minimized.

Copy link
Member Author

@fubuloubu fubuloubu commented Sep 11, 2019

So, you can format a string literal as bytes via .encode("utf-8"):

>>> "m12mm345mm".encode("utf-8")
b'm12mm345mm'
>>> "m12mm345mm{}".format("afgs").encode("utf-8")
b'm12mm345mmafgs'
>>> stuff = "afgafdg"
>>> f"m12mm345mm{stuff}".encode("utf-8")
b'm12mm345mmafgafdg'

Thank you for taking this on!

@fubuloubu

This comment has been minimized.

Copy link
Member Author

@fubuloubu fubuloubu commented Sep 11, 2019

As far as your questions, 1) whenever you can get it done by is fine, just set up a PR with "fixes #1567" in it (so it links back to this one) so we know what the progress is, and 2) not sure we have a style guide for how to write f-strings, if you have questions just ping us in the gitter. Thank you again!

@fubuloubu fubuloubu moved this from To do to In progress in Release Candidate! Sep 18, 2019
Release Candidate! automation moved this from In progress to Done Sep 23, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
2 participants
You can’t perform that action at this time.