Update ruff linters #213
Merged
Update ruff linters #213
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
Reviewer's GuideThis PR turns on all available Ruff lint rules and supplements the linting setup (including pyupgrade), updates the CI pre-commit hook to use ruff-check, refactors the codebase to satisfy the new lint constraints (removing obsolete abstract_method patterns and adjusting code style), introduces a test package initializer, and cleans up corresponding documentation. Class diagram for removal of AbstractMethod and related changesclassDiagram
class StatsCollector {
<<abstract>>
+collect()
+unproxied_stats()
+finish()
}
%% AbstractMethod was removed, so no longer shown as a base or used in exceptions
%% collect and unproxied_stats no longer raise AbstractMethod
class TraceCollector {
<<abstract>>
+trace_args()
+command(command: str)
}
%% AbstractMethod was removed, so no longer shown as a base or used in exceptions
%% trace_args no longer raises AbstractMethod
Class diagram for ShellLogger method renaming and log serializationclassDiagram
class ShellLogger {
+update_done_time()
+update_duration() %% was __update_duration
+print(msg: str, end: str = "\n")
+to_html() : Union[Iterator[str], list[Iterator[str]]]
+log(...)
+log_book
+done_time
+duration
}
class ShellLoggerJSONEncoder {
+default(obj: object) : object
}
ShellLoggerJSONEncoder --> ShellLogger : serializes
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}
There was a problem hiding this comment.
Hey @jmgate - I've reviewed your changes and they look great!
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location> `shell_logger/shell_logger.py:251` </location>
<code_context>
"""
self.done_time = datetime.now()
- def __update_duration(self) -> None:
+ def update_duration(self) -> None:
"""
Update the :attr:`duration` attribute.
</code_context>
<issue_to_address>
Renaming __update_duration to update_duration improves clarity and avoids name mangling.
This also improves accessibility for testing and aligns with standard Python naming for non-private methods.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
jmgate
force-pushed
the
update-ruff-linters
branch
from
5c8cea5 to
781371b
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #213 +/- ##
==========================================
+ Coverage 77.92% 78.67% +0.75%
==========================================
Files 7 6 -1
Lines 847 830 -17
Branches 145 143 -2
==========================================
- Hits 660 653 -7
+ Misses 147 138 -9
+ Partials 40 39 -1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
I think its existence was a historical artifact of starting this project before the @AbstractMethod decorator was available.
jmgate
force-pushed
the
update-ruff-linters
branch
from
781371b to
6ddc335
Compare
|
@sourcery-ai review |
There was a problem hiding this comment.
Hey @jmgate - I've reviewed your changes - here's some feedback:
- After removing
abstract_method.py, verify that all formerly custom abstract-method errors are now correctly enforced via@abstractmethodand that any tests expecting the oldAbstractMethodexception are updated to catchTypeErrorinstead. - The rename of
__update_durationtoupdate_durationis a breaking change—ensure no external code or serialization paths rely on the old private method name, or consider adding an alias for backwards compatibility. - In
stats_collectors, usingcollectors.extend(generator)works but is less self-documenting than a direct list comprehension; consider assigningcollectors = [collector(...) for collector in ...]to match the declared return type and improve readability.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- After removing `abstract_method.py`, verify that all formerly custom abstract-method errors are now correctly enforced via `@abstractmethod` and that any tests expecting the old `AbstractMethod` exception are updated to catch `TypeError` instead.
- The rename of `__update_duration` to `update_duration` is a breaking change—ensure no external code or serialization paths rely on the old private method name, or consider adding an alias for backwards compatibility.
- In `stats_collectors`, using `collectors.extend(generator)` works but is less self-documenting than a direct list comprehension; consider assigning `collectors = [collector(...) for collector in ...]` to match the declared return type and improve readability.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Sourcery suggested:
In stats_collectors, using collectors.extend(generator) works but is
less self-documenting than a direct list comprehension; consider
assigning collectors = [collector(...) for collector in ...] to
match the declared return type and improve readability.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Type: Task
Description
Trying to turn on all available Ruff linters.
Summary by Sourcery
Enable all available Ruff lint rules, adjust code to conform to them, and update tooling configuration accordingly.
Enhancements:
CI:
Documentation:
Tests: