-
Notifications
You must be signed in to change notification settings - Fork 0
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
Sourcery refactored master branch #1
base: master
Are you sure you want to change the base?
Conversation
if not isinstance(other, Batch): | ||
return False | ||
return other.reference == self.reference | ||
return other.reference == self.reference if isinstance(other, Batch) else False |
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.
Function Batch.__eq__
refactored with the following changes:
- Lift code into else after jump in control flow (
reintroduce-else
) - Swap if/else branches (
swap-if-else-branches
) - Replace if statement with if expression (
assign-if-exp
)
if other.eta is None: | ||
return True | ||
return self.eta > other.eta | ||
return True if other.eta is None else self.eta > other.eta |
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.
Function Batch.__gt__
refactored with the following changes:
- Lift code into else after jump in control flow (
reintroduce-else
) - Replace if statement with if expression (
assign-if-exp
)
if not result: | ||
return "not found", 404 | ||
return jsonify(result), 200 | ||
return ("not found", 404) if not result else (jsonify(result), 200) |
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.
Function allocations_view_endpoint
refactored with the following changes:
- Lift code into else after jump in control flow (
reintroduce-else
) - Replace if statement with if expression (
assign-if-exp
)
message = subscription.get_message(timeout=1) | ||
if message: | ||
if message := subscription.get_message(timeout=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.
Function test_change_batch_quantity_leading_to_reallocation
refactored with the following changes:
- Use named expression to simplify assignment and conditional (
use-named-expression
)
bus = bootstrap.bootstrap( | ||
yield bootstrap.bootstrap( | ||
start_orm=True, | ||
uow=unit_of_work.SqlAlchemyUnitOfWork(sqlite_session_factory), | ||
notifications=notifications.EmailNotifications(), | ||
publish=lambda *args: None, | ||
) | ||
yield bus |
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.
Function bus
refactored with the following changes:
- Inline variable that is immediately yielded (
inline-immediately-yielded-variable
)
assert rows == [] | ||
assert not rows |
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.
Function test_rolls_back_uncommitted_work_by_default
refactored with the following changes:
- Replaces an empty collection equality with a boolean operation (
simplify-empty-collection-comparison
)
|
||
|
||
def test_rolls_back_on_error(sqlite_session_factory): | ||
|
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.
Function test_rolls_back_on_error
refactored with the following changes:
- Replaces an empty collection equality with a boolean operation (
simplify-empty-collection-comparison
)
bus = bootstrap.bootstrap( | ||
yield bootstrap.bootstrap( | ||
start_orm=True, | ||
uow=unit_of_work.SqlAlchemyUnitOfWork(sqlite_session_factory), | ||
notifications=mock.Mock(), | ||
publish=lambda *args: None, | ||
) | ||
yield bus |
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.
Function sqlite_bus
refactored with the following changes:
- Inline variable that is immediately yielded (
inline-immediately-yielded-variable
)
f"Out of stock for POPULAR-CURTAINS", | ||
"Out of stock for POPULAR-CURTAINS" |
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.
Function TestAllocate.test_sends_email_on_out_of_stock_error
refactored with the following changes:
- Replace f-string with no interpolated values with string (
remove-redundant-fstring
)
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.
PR Type: Refactoring
Summary of PR: This PR introduces a series of refactoring changes aimed at simplifying and condensing the codebase. It utilizes more concise Python syntax such as ternary operators and the walrus operator to reduce the number of lines and improve the efficiency of the code.
General PR suggestions
- Consider the balance between code conciseness and readability. While ternary operators and inline assignments can shorten the code, they may also reduce clarity, especially in complex logical conditions.
- Review the use of guard clauses in comparison methods to ensure that the logic remains clear and maintainable.
- Evaluate the changes to ensure that they do not introduce any unintended side effects, particularly in the domain logic where the behavior of comparison operations is critical.
- Assess whether the refactoring aligns with the overall coding style and standards of the project, as consistency is key to maintainability.
- Verify that the refactoring does not negatively impact the performance of the application, especially in areas where the logic has been condensed.
Your trial expires on December 15, 2023. Please email [email protected] to continue using Sourcery ✨
if not isinstance(other, Batch): | ||
return False | ||
return other.reference == self.reference | ||
return other.reference == self.reference if isinstance(other, Batch) else False |
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.
suggestion (llm): Consider using the 'isinstance' check as the guard clause for clarity and to keep the happy path unindented.
return other.reference == self.reference if isinstance(other, Batch) else False | |
return isinstance(other, Batch) and other.reference == self.reference |
if other.eta is None: | ||
return True | ||
return self.eta > other.eta | ||
return True if other.eta is None else self.eta > other.eta |
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.
suggestion (llm): The ternary operation here makes the comparison less clear. Consider reverting to the original 'if' statements for better readability.
if not result: | ||
return "not found", 404 | ||
return jsonify(result), 200 | ||
return ("not found", 404) if not result else (jsonify(result), 200) |
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.
nitpick (llm): While the one-liner is concise, it may sacrifice some readability. Consider if the previous multi-line approach with explicit 'if' statement was clearer for readers.
Branch
master
refactored by Sourcery.If you're happy with these changes, merge this Pull Request using the Squash and merge strategy.
See our documentation here.
Run Sourcery locally
Reduce the feedback loop during development by using the Sourcery editor plugin:
Review changes via command line
To manually merge these changes, make sure you're on the
master
branch, then run:Help us improve this pull request!