WPS303: Allow underscore with 3 digits on the right#3128
WPS303: Allow underscore with 3 digits on the right#3128sobolevn merged 9 commits intowemake-services:masterfrom
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #3128 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 118 118
Lines 5788 5803 +15
Branches 801 804 +3
=========================================
+ Hits 5788 5803 +15 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| '100_000_000', | ||
| '"1_00"', | ||
| '"10_00"', | ||
| '"100_00"', |
There was a problem hiding this comment.
This is not correct. It should be 10_000
| million = 100_00_00 | ||
| big_number = 100000_0000_000 | ||
|
|
||
| .. versionadded:: 0.1.0 |
There was a problem hiding this comment.
Please, add .. versionchanged:: 1.0.0 with an explanation: what has changed.
| return string.lstrip("'").rstrip("'") | ||
|
|
||
|
|
||
| def _is_wrongly_underscored_number(string: str) -> bool: |
| '+1_000', | ||
| '+1_000.0', | ||
| '-1_000', | ||
| '-1_000.0', |
There was a problem hiding this comment.
There are also cases like 1.0_0, please test them.
|
@sobolevn, thank you for your feedback. In return, I am pleased to inform you that I have made the changes you requested. However, I have a few questions or comments:
|
| '"1.0_0"', | ||
| '"3_3.3"', | ||
| '"1_0_0"', | ||
| '"1_0_0_1"', |
There was a problem hiding this comment.
These numbers are not correct. Number can only considered correct if after split on _:
- First group has length of
1,2or3 - All other groups have length of strictly
3
The same with float numbers: we split on . and apply this rule to the float part as well:
0.0_005is correct100_000.000_001is correct0.00_05is not correct
There was a problem hiding this comment.
I will take part in your discussion.
is the number "1.0_0" considered a number?
or should it be interpreted as a string value, due to the fact used that there are quotes in quotes.
I was earlier to see a used shortened SemVer, an example: version = "2.4_15".
and it was just a string field.
There was a problem hiding this comment.
Oh, sorry. I missed these quotes. Please, add a comment that you also test strings.
| '-100_0', | ||
| '1000_000', | ||
| '-1000_000', | ||
| '1.0_0', |
There was a problem hiding this comment.
Please increase coverage of invalid float numbers. Add: 0_0.00_1 and 0.000_1
d054c3d to
a6aa1ce
Compare
|
@sobolevn , sorry for the mistake. I misunderstood the problem with floating point numbers. I've made edits and added more test cases. |
sobolevn
left a comment
There was a problem hiding this comment.
Thank you! Congrats on your first PR :)
Checklist
CHANGELOG.mdRelated issues
WPS303: it should only be possible to write_with 3 digits on the right #3120This is my first pull request to this project, so please bear with me if I made any mistakes. Any comments or suggestions are welcome.