🧹 clean up and upgrade python syntax.

[wiseaidev]

Signed-off-by: Harmouch101 [email protected]

What was wrong?

Related to Issue #

How was it fixed?

As of python3, 'Unicode' strings are the default. Hence no need for the prefix u. Similarly, __nonzero__ is a python2 syntax for defining boolness. I also used f-strings since this project support 3.6+.

So, I manually went through the codebase, updated:

• old-school string formatting to f-strings.
• list to set whenever possible because sets are more efficient. to avoid redundant calls of set.
• Some other cleanups.
• Some of the error messages:

        message = (
            f"\nCould not identify the intended function with name `"
            f"{fn_identifier}`, positional argument"
            f"{'s' if len(args) > 1 else ''} of type "
            f"`{tuple(map(type, args))}` and keyword argument"
            f"{'s' if len(kwargs) > 1 else ''} of type "
            f"`{valmap(type, kwargs)}`.\nFound {len(matching_identifiers)} "
            f"function{'s' if len(matching_identifiers) > 1 else ''} with "
            f"the name `{fn_identifier}`"
            f": {matching_function_signatures}{diagnosis}"
        )

output example:

"\nCould not identify the intended function with name `setBytes32Value`, positional argument of type `(<class 'str'>,)` and keyword argument of type `{}`.\nFound 1 function with the name `setBytes32Value`: ['setBytes32Value(bytes32[])']\nFunction invocation failed due to no matching argument types.".

Todo:

• Add entry to the release notes

Cute Animal Picture

🐶

[wiseaidev]

Apparently, f-strings seem to be the fastest string formatting method in microbenchmarks as Raymond Hettinger showed.

[wiseaidev]

I think this PR is ready for reviews from the warriors @fselmo, @kclowes, @pipermerriam ...

[fselmo]

Hey @Harmouch101. Wow. Thanks for the hefty sweeping! I think in the future it might be nicer to break the different changes up into separate PRs for large loads like this but bravo 👍 and thank you 🙏 . We'd been steering towards f-strings in general, changing it where it made sense, and this certainly helps us get there faster 😅.

I left some nit comments where it's nice that f-strings are shorter since we can fit messages in one line rather than line split.

I think I found just one misrepresentation / typo from the old .format() to f-string format that I didn't preface with a "nit:" tag.

I left a comment on the pluralization logic for strings in general. I think that though the pluralization being accurate is nice, I'm not sure that outweighs the same logic being repeated across all strings in the code base where things might need to be plural. I think it might be better to have static messages in general too with just the expected variables being dynamic - for consistency to the end-user.

I could've totally overlooked something too so let me know. Thanks again though for another very thorough PR.

---

edit: I forgot to mention it but I'm a big fan of the list -> set changes where it made sense 👌

[kclowes]

LGTM after the comments from @fselmo are addressed! Thanks @Harmouch101!

[wiseaidev]

@fselmo, Just ping to let you know I have done the changes requested.

[fselmo]

@fselmo, Just ping to let you know I have done the changes requested.

Just getting back to this. Thanks again for these changes @Harmouch101 👌

[wiseaidev]

Hey @fselmo , just a suggestion about the commit messages. I think for the future it would be better to squash all commits into a single one cause some of my commit messages doesn't make sense for the whole code base, rather for this specific PR. Therefore, i suggest that the contribution file should mention that the authors of PRs must squash their commits when they finish pushing all the necessary code for a specific PR. Or, actually, to make life easier for new contributors, the task can be done by maintainers.

Regards.

[fselmo]

Hey @Harmouch101. Agreed. We usually do squash commits. I overlooked it before merging this time.