This repository was archived by the owner on Sep 23, 2025. It is now read-only.
Merge from master #646
Merged
Merge from master #646
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Merge from master
The asteval module allows you to evaluate a large subset of the Python language from within a python program, without using eval(). It is, in effect, a restricted version of Python’s built-in eval(), forbidding several actions, and using (by default) a simple dictionary as a flat namespace.
timeout-minutes: 10
Merge from master
./kyu_6/disease_spread/epidemic.py:48:33: W504 line break after binary operator
susceptible[k] - dt *
^
./kyu_6/disease_spread/epidemic.py:49:25: W504 line break after binary operator
kwargs['b'] *
^
./kyu_6/disease_spread/epidemic.py:50:28: W504 line break after binary operator
susceptible[k] *
^
./kyu_6/disease_spread/epidemic.py:54:31: W504 line break after binary operator
infecteds[k] + dt *
^
./kyu_6/disease_spread/epidemic.py:55:26: W504 line break after binary operator
(kwargs['b'] *
^
./kyu_6/disease_spread/epidemic.py:56:29: W504 line break after binary operator
susceptible[k] *
^
./kyu_6/disease_spread/epidemic.py:57:41: W504 line break after binary operator
infecteds[k] - kwargs['a'] *
Run python -m flake8 ./kyu_6 --count --benchmark --show-source --statistics
./kyu_6/disease_spread/epidemic.py:48:17: E126 continuation line over-indented for hanging indent
susceptible[k] - dt * kwargs['b'] * susceptible[k] * infecteds[k]
^
./kyu_6/disease_spread/epidemic.py:53:17: E126 continuation line over-indented for hanging indent
infecteds[k] + dt *
^
./kyu_6/disease_spread/epidemic.py:53:35: W504 line break after binary operator
infecteds[k] + dt *
^
Run python -m flake8 ./kyu_6 --count --benchmark --show-source --statistics
./kyu_6/disease_spread/epidemic.py:53:31: W504 line break after binary operator
infecteds[k] + dt *
^
Run python -m flake8 ./kyu_6 --count --benchmark --show-source --statistics
./kyu_6/potion_class_101/potion.py:52:24: W503 line break before binary operator
* self.volume) / (other.volume + self.volume))
^
./kyu_6/potion_class_101/potion.py:54:24: W503 line break before binary operator
* self.volume) / (other.volume + self.volume))
^
./kyu_6/potion_class_101/potion.py:56:24: W503 line break before binary operator
* self.volume) / (other.volume + self.volume))
^
3 W503 line break before binary operator
0.313 seconds elapsed
Run python -m flake8 ./kyu_6 --count --benchmark --show-source --statistics
./kyu_6/potion_class_101/potion.py:52:75: W504 line break after binary operator
(other.color[0] * other.volume + self.color[0] * self.volume) /
^
./kyu_6/potion_class_101/potion.py:56:75: W504 line break after binary operator
(other.color[1] * other.volume + self.color[1] * self.volume) /
^
./kyu_6/potion_class_101/potion.py:60:75: W504 line break after binary operator
(other.color[2] * other.volume + self.color[2] * self.volume) /
^
3 W504 line break after binary operator
0.319 seconds elapsed
Run python -m flake8 ./kyu_6 --count --benchmark --show-source --statistics ./kyu_6/potion_class_101/potion.py:10:1: F401 'typing.ClassVar' imported but unused from typing import Tuple, ClassVar ^ 1 F401 'typing.ClassVar' imported but unused 0.32 seconds elapsed
./kyu_6/potion_class_101/potion.py:63 in private method `__calc_rgb`:
D205: 1 blank line required between summary line and description (found 0)
Multi-line docstrings consist of a summary line just like a one-line
docstring, followed by a blank line, followed by a more elaborate
description. The summary line may be used by automatic indexing tools;
it is important that it fits on one line and is separated from the
rest of the docstring by a blank line.
Run python -m flake8 ./kyu_6 --count --benchmark --show-source --statistics
./kyu_6/potion_class_101/potion.py:65:1: W293 blank line contains whitespace
"""
Calculate RGB values.
:param index: int
:param other: Object
:param total_volume: int
:return:
"""
^
1 W293 blank line contains whitespace
Merge pull request #644 from iKostanOrg/master
Contributor
Reviewer's Guide by SourceryThis pull request refactors the codebase by introducing helper functions for condition checks, optimizes the workflow configurations, and enhances the test coverage. It also includes the addition of a new feature for replacing letters with their alphabet positions. Sequence diagram for mixing two potionssequenceDiagram
participant Potion1
participant Potion2
Potion1->>Potion1: mix(Potion2)
Potion1->>Potion1: __calc_rgb(Potion2, new_volume)
Potion1->>Potion2: other.color
Potion1->>Potion2: other.volume
Potion1-->>Potion1: r, g, b
Potion1-->>Potion1: Potion((r, g, b), new_volume)
Potion1-->>Potion1: return new Potion
Updated class diagram for Potion ClassclassDiagram
class Potion {
- Tuple[int, int, int] color
- int volume
+ __init__(color: Tuple[int, int, int], volume: int)
+ mix(other: Potion) : Potion
+ volume() : int
+ volume(value: int) : None
+ color() : Tuple[int, int, int]
- __calc_rgb(other: Potion, new_volume: int) : Tuple[int, int, int]
}
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Hey @ikostan - I've reviewed your changes - here's some feedback:
Overall Comments:
- Consider using more descriptive names for functions like
get_condition_0to improve readability. - The battleship validator could be simplified by extracting the ship counting logic into a separate function.
- The addition of ASTEVAL in the basic math problem is a good way to avoid security issues with eval.
Here's what I looked at during the review
- 🟡 General issues: 1 issue found
- 🟢 Security: all looks good
- 🟡 Testing: 2 issues found
- 🟡 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.
kyu_6/replace_with_alphabet_position/test_replace_with_alphabet_position.py
Outdated
Show resolved
Hide resolved
| row = coordinates['row'] | ||
| col = coordinates['col'] | ||
|
|
||
| if col == len(spiral[0]) - 1 and \ |
Contributor
There was a problem hiding this comment.
issue (complexity): Consider inlining the helper functions and merging similar conditional checks to reduce fragmentation and improve readability.
You can reduce fragmentation by inlining the trivial conditions with clear inline comments instead of using six tiny helper functions. For example, instead of writing:
if get_condition_0(spiral, row, col):
spiral[row][col] = 1
coordinates['col'] += 1
done = False
breakyou could consolidate that logic right in the main function:
# Condition 0: if next cell is the last in the row and is empty, move and mark current cell.
if col + 1 == len(spiral[0]) - 1 and spiral[row][col + 1] == spiral[row][col] == 0:
spiral[row][col] = 1
coordinates['col'] += 1
done = False
breakSimilarly, if multiple conditions have similar structure, merge them into a single if/elif block with comments. For example:
# Check conditions for breaking (conditions 1 to 3)
if (col == len(spiral[0]) - 1 and spiral[row][col] == 0) or \
(col + 2 == len(spiral[0]) - 1 and spiral[row][col + 2] == 1 and
spiral[row][col + 1] == spiral[row][col] == 0) or \
(col + 2 < len(spiral[0]) - 1 and spiral[row][col + 2] == 1 and
spiral[row][col + 1] == spiral[row][col] == 0 and
col + 2 < len(spiral[0]) and spiral[row + 1][col] != 1):
spiral[row][col] = 1
done = False
breakThis approach keeps the functionality intact while reducing extra indirections and making the core logic easier to follow.
❌ 61 blocking issues (72 total)
@qltysh one-click actions: |
suggestion (testing): Add more test cases for edge cases and non-alpha characters
It would be beneficial to include test cases with special characters like "!@#$%^&*()_+=-`~[]{}|;':",./<>?", numbers, and a mix of alphanumeric and special characters to ensure comprehensive test coverage.
Suggested implementation:
@parameterized.expand([
("", ""),
("123", ""),
("!@#$%^&*()", ""),
("a1!b", "1 2"),
("AbZ!", "1 2 26"),
])
def test_alphabet_position(self, string, expected):
"""
Testing 'alphabet_position' function with various edge cases including empty strings,
numbers, special characters, and mixed alphanumeric input.
"""
# pylint: disable-msg=R0801
allure.dynamic.title("Testing the 'alphabet_position' with edge cases")
Ensure that the expected results match the behavior of the alphabet_position function in cases involving non-alpha characters. If the function behavior is different from assumed here, adjust the expected outputs accordingly.
suggestion (testing): Missing tests for invalid 1x1 Sudoku
While there's a test for a valid 1x1, there are no tests for invalid 1x1 scenarios (e.g., with values other than 1 or with non-integer values). Add tests to cover these cases.
Suggested change
([[1]], True, 'Testing valid 1x1'),
([[1]], True, 'Testing valid 1x1'),
([[2]], False, 'Testing invalid 1x1 with wrong value'),
(([['a']], False, 'Testing invalid 1x1 with non-integer value'),
ikostan
added a commit
that referenced
this pull request
Feb 16, 2025
Merge pull request #646 from iKostanOrg/master
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary by Sourcery
Refactor code for clarity and maintainability, fix several bugs, and add a new kata solution for 'Replace With Alphabet Position'. Update the CI pipeline to improve code quality and testing.
New Features:
Bug Fixes:
make_spiralfunction where the spiral was not being generated correctly.validate_battlefieldfunction where some valid battlefields were being rejected.calculatefunction where additional spaces caused incorrect calculationsEnhancements:
make_spiralfunction by optimizing the conditions.validate_battlefieldfunction by simplifying the logic.calculatefunction by using ASTEVAL.potionclass by adding type hints.epidemicfunction by using more descriptive variable names.sol_equafunction.test_generate_hashtagandtest_unique_in_order.test_tickets.how_many_dalmatiansfunction.Documentation:
how_many_dalmatiansfunction.Tests:
test_sudoku.py.test_string_transformer.py,test_has_subpattern.py, andtest_tickets.pyto use multiline strings.test_generate_hashtag.pyandtest_unique_in_order.pyfor better clarity.test_calculate_damage.pyfor consistency.test_replace_with_alphabet_position.py.Chores:
make_spiral.validate_battlefieldusingall().test_sudoku.py.mainbranch.pull_request_targetevents.utils.Documentationbranch.potionclass to include type hints.epidemicfunction to use more descriptive variable names and simplify the calculations.sol_equafunction to simplify the starting value calculation.calculate_damagefunction to use consistent spacing.warriorclass to simplify the experience update logic.Sudokuclass to simplify the data validation logic.string_transformerandhas_subpatterntest cases to use multiline strings.ticketstest case to use multiline strings.how_many_dalmatiansfunction.