-
Notifications
You must be signed in to change notification settings - Fork 6
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
Pydocstyle + Merge pull request #550 from iKostanOrg/master #551
Conversation
Merge from master
Reviewer's Guide by SourceryThis PR primarily focuses on improving code documentation and style in the kyu_4 directory. The changes include adding and enhancing docstrings across multiple Python files, standardizing docstring formats, and adding package-level documentation. A new GitHub workflow for pydocstyle checking has also been added. No diagrams generated as the changes look simple and do not need a visual representation. File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
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.
We have skipped reviewing this pull request. Here's why:
- We don't review packaging changes - Let us know if you'd like us to change this.
- All of the files are larger than we can process. We're working on it!
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #551 +/- ##
=======================================
Coverage 90.41% 90.41%
=======================================
Files 172 172
Lines 2577 2577
=======================================
Hits 2330 2330
Misses 247 247 ☔ View full report in Codecov by Sentry. |
Code Climate has analyzed commit fc29afe and detected 0 issues on this pull request. The test coverage on the diff in this pull request is 92.7% (50% is the threshold). This pull request will bring the total coverage in the repository to 90.4% (0.0% change). View more on Code Climate. |
@sourcery-ai review |
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.
Hey @ikostan - I've reviewed your changes - here's some feedback:
Overall Comments:
- Consider standardizing the docstring format across all functions - some have a newline after the first line while others don't. Pick one style and apply it consistently.
Here's what I looked at during the review
- 🟢 General issues: all looks good
- 🟢 Security: all looks good
- 🟢 Testing: all looks good
- 🟡 Complexity: 1 issue found
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
@@ -67,34 +71,36 @@ def format_duration(seconds: int) -> str: | |||
return result | |||
|
|||
|
|||
def format_days(days: int, day: str, result: str) -> str: | |||
def format_days(days: int, |
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.
issue (complexity): Consider consolidating the format_days, format_hours, format_minutes and format_seconds functions into a single helper function.
The formatting functions can be consolidated into a single helper function to reduce duplication while maintaining all functionality. Here's how:
def format_component(value: int,
component: str,
result: str,
is_final: bool = False,
has_seconds: bool = False) -> str:
"""Format a time component with appropriate conjunction.
Args:
value: The time value
component: The formatted string (e.g. "1 day")
result: The accumulated result string
is_final: Whether this is the final component
has_seconds: Whether seconds exist (for minute special case)
"""
if value > 0:
if not result:
return component
if is_final or (has_seconds and component.startswith('1 minute')):
return f'{result} and {component}'
return f'{result}, {component}'
return result
# Usage example:
result = format_component(days, day, result)
result = format_component(hours, hour, result)
result = format_component(minutes, minute, result, seconds == 0, seconds > 0)
result = format_component(seconds, second, result, True)
This consolidates the duplicate logic while handling the special cases for minutes and final components. The code becomes more maintainable as formatting changes only need to be made in one place.
./kyu_4/the_greatest_warrior/test_battle.py:39:1: W293 blank line contains whitespace """ Testing Battle method with various test data. :return: """ ^ ./kyu_4/the_greatest_warrior/test_battle.py:40:17: W291 trailing whitespace """ Testing Battle method with various test data. :return: """
./kyu_4/strings_mix/solution.py:16:1: W293 blank line contains whitespace """ Mix function. Given two strings s1 and s2, we want to visualize how different the two strings are. We will only take into account the lowercase letters (a to z). First let us count the frequency of each lowercase letters in s1 and s2. :param s1: string a :param s2: string b :return: the difference between two strings """
./kyu_4/range_extraction/solution.py:12:1: W293 blank line contains whitespace """ Solution for Range Extraction problem. Tt takes a list of integers in increasing order and returns a correctly formatted string in the range format. :param args: list :return: str """ ^ ./kyu_4/range_extraction/solution.py:41:19: W291 trailing whitespace def case_3(a: int, ^ ./kyu_4/range_extraction/solution.py:42:26: W291 trailing whitespace current: list, ^ ./kyu_4/range_extraction/solution.py:46:1: W293 blank line contains whitespace """ Case #3. :param a: int :param current: list :param result: str :return: str """ ^ ./kyu_4/range_extraction/solution.py:67:1: W293 blank line contains whitespace """ Case #2. :return: str """
kyu_4/next_bigger_number_with_the_same_digits/next_bigger.py:39 in public function `digit_that_breaks_ordering_index`: D401: First line should be in imperative mood (perhaps 'Find', not 'Finds') [Docstring] prescribes the function or method's effect as a command: ("Do this", "Return that"), not as a description; e.g. don't write "Returns the pathname ...".
Merge from master
Summary by Sourcery
Enhance code documentation by improving docstring formatting and parameter annotations. Introduce a new CI workflow using pydocstyle to enforce consistent docstring styles. Add init.py files to various directories to establish them as Python packages.
Enhancements:
CI:
Chores: