GeezerVisonThe World Wide Web, restored to 800 pixels of glory. | |||||||||||||||||||||||||||||||||||||||
| Home|About|Gaming cabinet|Archives|How it works|Starter sites | |||||||||||||||||||||||||||||||||||||||
GeezerVison is showing github.com — gh-109373: Store metadata required for pystats comparison in the json by mdboom · Pull Request #109374 · python/cpython Conversation
|
| def ratio_to_float(s): | ||
| """ | ||
| The inverse of format_ratio. | ||
| """ | ||
| if s == "": | ||
| return 0.0 | ||
| else: | ||
| return float(s[:-1]) | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This confused me, because it's not the inverse, right? It just turns "4.2%" into 4.2 (which feels wrong to me since I'd expect .042... I know that this is what the old code does, and it's only used as a sort key, so I guess either is fine).
So I'd suggest changing the name and docstring, but I honestly can't think of a good name for this operation...
At least, if this is gonna be its own function, maybe assert s.endswith("%") so it's clear what this function is doing?
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
All reactions
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point. I renamed the function and clarified in the docstring.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
All reactions
| stats[key] += value | ||
| stats['__nfiles__'] += 1 | ||
|
|
||
| import opcode |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just curious, why was this import moved here? Just to reduce the temptation to use it elsewhere in the file?
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
All reactions
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, exactly. Any use of opcode, dis or other sorts of bytecode introspection things on the "compare two results" path could lead to a subtle, silent error. I'll add a comment.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
All reactions
Reviewers
Uh oh!
There was an error while loading. Please reload this page.
Assignees
No one assignedLabels
Projects
Milestone
No milestoneDevelopment
Successfully merging this pull request may close these issues.
Uh oh!
There was an error while loading. Please reload this page.
2 participants
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.