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.

Sign up for GitHub

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

Truncate long local variable values with unittest utils #109

Closed
wants to merge 4 commits into from
Closed

Truncate long local variable values with unittest utils #109

Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
4700b9b
Truncate long local variable values with unittest utils
vegarsti Apr 18, 2019
f1f1082
Truncate one-line values with safe_repr with short keyword instead
vegarsti Apr 24, 2019
3296f32
Truncate ,ultiline values line by line as oneline
vegarsti Apr 24, 2019
691f28c
Use better variable names and remove nested function
vegarsti Apr 25, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 8 additions & 3 deletions marbles/core/marbles/core/marbles.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -330,13 +330,18 @@ def formattedMsg(self): # mimic unittest's name for standardMsg

@classmethod
def _format_local(cls, name, value):
def truncate(s):
return unittest.util.safe_repr(s, short=True)
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I ended up doing this to make the lines shorter. Not sure if this is good or not.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I prefer not defining nested functions like this. There are a couple of other ways to shorten the lines here:

  1. Import unittest.util directly so you can call util.safe_repr (which is a good deal shorter than unittest.util.safe_repr)
from unittest import util
  1. Use line breaking and even more concise variable names!
short_lines = [unittest.util.safe_repr(line, short=True)
               for line in value_str.split('\n')]
short_value_str = '\n'.join(short_lines)
value_str = textwrap.indent(short_value_str, '\t\t')

I want to think more about what exactly we want to do with multiline locals, but since you asked about this piece of code specifically I figured I'd show you these options.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great, that makes sense. Thanks!
Are there conventions on when/how to line break? I know many use black to auto format the code.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PEP8 has a section about breaking long lines, but it doesn't have a ton of examples. The Hitchiker's Guide to Python has a short section on Line Continuations, too, but again, not a ton of examples.

One of the best things you can do is simply to read Python code and see how long lines are broken. You're more than welcome to read more of marbles the source code to get a sense of our line-breaking style.

You can also use black, and see how it breaks up long lines in different scenarios. We don't use black to format marbles, so running black on the whole codebase may change a lot of things outside of the specific code you're changing here.

Copy link
Author

@vegarsti vegarsti Apr 25, 2019 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Got it, thanks!

value_str = repr(value)
if '\n' in value_str:
value_str = textwrap.indent(value_str, '\t\t')
value_str_lines = value_str.split('\n')
value_str_lines = [truncate(line) for line in value_str_lines]
truncated_value_str = '\n'.join(value_str_lines)
value_str = textwrap.indent(truncated_value_str, '\t\t')
return '\t{0} =\n{1}'.format(name, value_str)
else:
short_value_str = unittest.util.safe_repr(value_str, short=True)
return '\t{0} = {1}'.format(name, short_value_str)
value_str = truncate(value_str)
return '\t{0} = {1}'.format(name, value_str)

@classmethod
def _format_locals(cls, locals_):
Expand Down