-
Notifications
You must be signed in to change notification settings - Fork 101
style: Enforce ruff/flake8-comprehensions rules (C4) #2090
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
Conversation
Unnecessary generator (rewrite as a list comprehension) I further simplified the code manually.
Unnecessary generator (rewrite as a set comprehension)
Unnecessary list literal (rewrite as a set literal)
Unnecessary `dict()` call (rewrite as a literal)
Unnecessary list comprehension (rewrite using `list()`)
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.
I can be happy about this change even if it touches our code-styling, since it is added and applied by ruff and therefore standardizes the code.
What do you think @auguste-probabl? I know that, under certain conditions, you prefer the dict constructor to {}.
Coverage Report for |
|||||||||||||||||||||||||||||||||||||||||||||
| File | Stmts | Miss | Cover | Missing |
|---|---|---|---|---|
| skore-local-project/src/skore_local_project | ||||
| __init__.py | 5 | 0 | 100% | |
| metadata.py | 87 | 4 | 95% | 26, 38, 150–151 |
| project.py | 102 | 1 | 99% | 244 |
| storage.py | 40 | 2 | 95% | 45, 187 |
| TOTAL | 234 | 7 | 97% | |
| Tests | Skipped | Failures | Errors | Time |
|---|---|---|---|---|
| 24 | 0 💤 | 0 ❌ | 0 🔥 | 5.755s ⏱️ |
Coverage Report for |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| File | Stmts | Miss | Cover | Missing |
|---|---|---|---|---|
| skore-hub-project/src/skore_hub_project | ||||
| __init__.py | 22 | 1 | 95% | 39 |
| protocol.py | 29 | 0 | 100% | |
| skore-hub-project/src/skore_hub_project/artifact | ||||
| __init__.py | 0 | 0 | 100% | |
| artifact.py | 23 | 0 | 100% | |
| serializer.py | 26 | 0 | 100% | |
| upload.py | 36 | 4 | 88% | 175, 177–178, 180 |
| skore-hub-project/src/skore_hub_project/artifact/media | ||||
| __init__.py | 5 | 0 | 100% | |
| data.py | 22 | 0 | 100% | |
| feature_importance.py | 34 | 0 | 100% | |
| media.py | 10 | 0 | 100% | |
| model.py | 9 | 0 | 100% | |
| performance.py | 44 | 0 | 100% | |
| skore-hub-project/src/skore_hub_project/artifact/pickle | ||||
| __init__.py | 2 | 0 | 100% | |
| pickle.py | 24 | 0 | 100% | |
| skore-hub-project/src/skore_hub_project/authentication | ||||
| __init__.py | 0 | 0 | 100% | |
| login.py | 66 | 0 | 100% | |
| logout.py | 4 | 0 | 100% | |
| token.py | 24 | 0 | 100% | |
| uri.py | 18 | 0 | 100% | |
| skore-hub-project/src/skore_hub_project/client | ||||
| __init__.py | 0 | 0 | 100% | |
| client.py | 60 | 0 | 100% | |
| skore-hub-project/src/skore_hub_project/metric | ||||
| __init__.py | 10 | 0 | 100% | |
| accuracy.py | 35 | 0 | 100% | |
| brier_score.py | 35 | 0 | 100% | |
| log_loss.py | 35 | 0 | 100% | |
| metric.py | 49 | 1 | 97% | 25 |
| precision.py | 61 | 0 | 100% | |
| r2.py | 35 | 0 | 100% | |
| recall.py | 63 | 0 | 100% | |
| rmse.py | 35 | 0 | 100% | |
| roc_auc.py | 35 | 0 | 100% | |
| timing.py | 90 | 4 | 95% | 52–53, 115–116 |
| skore-hub-project/src/skore_hub_project/project | ||||
| __init__.py | 0 | 0 | 100% | |
| project.py | 102 | 3 | 97% | 204, 308, 338 |
| skore-hub-project/src/skore_hub_project/report | ||||
| __init__.py | 3 | 0 | 100% | |
| cross_validation_report.py | 64 | 1 | 98% | 204 |
| estimator_report.py | 12 | 0 | 100% | |
| report.py | 52 | 0 | 100% | |
| TOTAL | 1174 | 14 | 98% | |
| Tests | Skipped | Failures | Errors | Time |
|---|---|---|---|---|
| 139 | 0 💤 | 0 ❌ | 0 🔥 | 1m 31s ⏱️ |
glemaitre
left a comment
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.
I'm fine with those changes. A bit strict with the dict but why not be consistent.
I wonder whether you would be interested in adding a few ruff rule sets.
For example, I find
C4rules improve the code. Some of them, such as ruleC408might be discutable - I would be happy to ignore such rules.