add more types #1222
Merged
add more types #1222
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 systematically introduces PEP 484/PEP 604 type hints throughout the codebase—annotating parameters, return types, union types, and container types—while applying minor logic refinements for clarity and consistency. Class diagram for updated type annotations in UpgradeDialog, UpgradeConfig, UpgradeTemplates, and VersionclassDiagram
class UpgradeDialog {
__init__(parent: QWidget | None = None)
fill_it_in(oldversion: nowplaying.upgradeutils.Version, newversion: nowplaying.upgradeutils.Version) -> None
}
class UpgradeConfig {
__init__(testdir: str | pathlib.Path | None = None)
_getconfig() -> QSettings
backup_config() -> None
upgrade() -> None
_oldkey_to_newkey(oldconfig: QSettings, newconfig: QSettings, mapping: dict[str, str]) -> None
}
class UpgradeTemplates {
copied: list[str]
oldshas: dict[str, dict[str, str]]
__init__(bundledir: str | pathlib.Path | None = None, testdir: str | pathlib.Path | None = None)
preload() -> None
check_preload(filename: str, userhash: str) -> str | None
setup_templates() -> None
_process_template_directory(app_dir: str | pathlib.Path, user_dir: pathlib.Path) -> None
}
class Version {
__init__(version: str)
_calculate() -> None
is_prerelease() -> bool
__str__() -> str
__lt__(other: t.Any) -> bool
__le__(other: t.Any) -> bool
__eq__(other: t.Any) -> bool
__ne__(other: t.Any) -> bool
__hash__() -> int
__gt__(other: t.Any) -> bool
__ge__(other: t.Any) -> bool
}
Class diagram for SubprocessManager with updated type annotationsclassDiagram
class SubprocessManager {
config: nowplaying.config.ConfigFile | None
testmode: bool
processes: dict[str, dict[str, t.Any]]
__init__(config: nowplaying.config.ConfigFile | None = None, testmode: bool = False)
start_all_processes(startup_window: 'nowplaying.startup.StartupWindow')
stop_all_processes() -> None
_start_process(processname: str) -> None
_stop_process_parallel(processname: str) -> None
_stop_process(processname: str) -> None
start_process(processname: str) -> None
stop_process(processname: str) -> None
restart_process(processname: str) -> None
start_webserver() -> None
start_kickbot() -> None
start_twitchbot() -> None
stop_webserver() -> None
stop_twitchbot() -> None
stop_kickbot() -> None
restart_webserver() -> None
restart_obsws() -> None
restart_kickbot() -> None
}
Class diagram for PluginVerifyError with updated type annotationclassDiagram
class PluginVerifyError {
message: str | None
__init__(message: str | None = None)
}
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 @aw-was-here - I've reviewed your changes and found some issues that need to be addressed.
Blocking issues:
- The new gt implementation may cause incorrect results. (link)
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location> `nowplaying/upgradeutils.py:119` </location>
<code_context>
self.chunk.get('rc'),
self.chunk.get('commitnum')))
- def __gt__(self, other):
+ def __gt__(self, other: t.Any) -> bool:
''' version greater than '''
- return not (self < other or self == other)
</code_context>
<issue_to_address>
The new __gt__ implementation may cause incorrect results.
Using 'self >= other and self != other' creates logical inconsistencies and risks infinite recursion, as __ge__ and __gt__ may call each other. Please revert to the standard implementation to ensure correct behavior.
</issue_to_address>
### Comment 2
<location> `nowplaying/upgradeutils.py:124` </location>
<code_context>
+ return self >= other and self != other
- def __ge__(self, other):
+ def __ge__(self, other: t.Any) -> bool:
''' version greater than or equal '''
return self == other or self > other
</code_context>
<issue_to_address>
The __ge__ method implementation can cause infinite recursion.
Mutual recursion between __ge__ and __gt__ can cause a stack overflow. Please refactor to prevent this.
</issue_to_address>
### Comment 3
<location> `nowplaying/bootstrap.py:47` </location>
<code_context>
app.setApplicationName(appname)
-def setuplogging(logdir=None, logname='debug.log', rotate=False):
+def setuplogging(logdir: pathlib.Path | str | None = None,
+ logname: str ='debug.log',
+ rotate: bool = False) -> pathlib.Path:
</code_context>
<issue_to_address>
The besuretorotate variable is redundant.
Since besuretorotate is assigned but never used, please remove it to clean up the code.
Suggested implementation:
```python
if logdir:
logpath = pathlib.Path(logdir)
if logpath.is_file():
logpath.mkdir(parents=True, exist_ok=True)
logfile = logpath.joinpath(logname)
```
If there is a line in the function assigning `besuretorotate = ...`, it should be removed as well. Since the provided code snippet does not show such a line, please ensure that any assignment to `besuretorotate` within the full function is deleted.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1222 +/- ##
==========================================
- Coverage 66.69% 66.68% -0.02%
==========================================
Files 68 68
Lines 11522 11527 +5
==========================================
+ Hits 7685 7687 +2
- Misses 3837 3840 +3
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
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.
Summary by Sourcery
Enhancements: