Skip to content

style: Enforce RUF010 and RUF015 rules #2091

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 6 commits into from
Oct 28, 2025
Merged

style: Enforce RUF010 and RUF015 rules #2091

merged 6 commits into from
Oct 28, 2025

Conversation

@DimitriPapadopoulos
Copy link
Contributor

@DimitriPapadopoulos DimitriPapadopoulos commented Oct 27, 2025 •
edited by thomass-dev
Loading

Enforce RUF010 and RUF015 rules to standardize the code base.

@DimitriPapadopoulos DimitriPapadopoulos changed the title Apply assorted ruff rules (RUF) style: Apply assorted ruff rules (RUF) Oct 27, 2025
@DimitriPapadopoulos DimitriPapadopoulos marked this pull request as ready for review October 28, 2025 14:20
@DimitriPapadopoulos
style: Apply ruff rule RUF003
3cf7441 
Comment contains ambiguous ` ` (NO-BREAK SPACE). Did you mean ` ` (SPACE)?
@DimitriPapadopoulos
style: Apply ruff rule RUF010
f921322 
Use explicit conversion flag
@DimitriPapadopoulos DimitriPapadopoulos force-pushed the RUF branch 2 times, most recently from 6d64af4 to 2091e4f Compare October 28, 2025 14:23
@DimitriPapadopoulos
style: Apply ruff rule RUF015
bf596cb 
Prefer `next(iter())` over single element slice
@DimitriPapadopoulos
style: Apply ruff rule RUF100
f88d097 
Unused `noqa` directive
@thomass-dev
Copy link
Collaborator

thomass-dev commented Oct 28, 2025 •
edited
Loading

I think you have spotted good points:

  • {self!r} instead of repr(self),
  • next(iter(...)) instead of list(...)[0].

Please add the related rules to ruff to enforce it.

thomass-dev
thomass-dev reviewed Oct 28, 2025
View reviewed changes
sphinx/conf.py Outdated
Comment on lines 17 to 18
from github_link import make_linkcode_resolve # noqa
from matplotlib_skore_scraper import matplotlib_skore_scraper # noqa
from github_link import make_linkcode_resolve
from matplotlib_skore_scraper import matplotlib_skore_scraper
Copy link
Collaborator

@thomass-dev thomass-dev Oct 28, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This directory is actually not covered by .pre-commit.
Please revert, otherwise i think we will have red IDE with some alerts on non-top imports.

Copy link
Contributor Author

@DimitriPapadopoulos DimitriPapadopoulos Oct 28, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I ran ruff manually instead of pre-commit.

By the way, which tool does your IDE use to lint and raise such alerts? I think the relevant ruff alert would be E402 from pycodestyle, but the IDE might run a different linter.

@DimitriPapadopoulos
Copy link
Contributor Author

DimitriPapadopoulos commented Oct 28, 2025

Please add the related rules to ruff to enforce it.

Would you prefer I add all RUF rules and ignore some of them, or add only RUF010 and RUF015 for now?

@thomass-dev
Copy link
Collaborator

thomass-dev commented Oct 28, 2025

Please add the related rules to ruff to enforce it.

Would you prefer I add all RUF rules and ignore some of them, or add only RUF010 and RUF015 for now?

Only the ones you mention for now.

@probabl-ai probabl-ai deleted a comment from github-actions bot Oct 28, 2025
@probabl-ai probabl-ai deleted a comment from github-actions bot Oct 28, 2025
@probabl-ai probabl-ai deleted a comment from github-actions bot Oct 28, 2025
@DimitriPapadopoulos
Copy link
Contributor Author

DimitriPapadopoulos commented Oct 28, 2025 •
edited
Loading

By the way, T rules are not documented:
https://docs.astral.sh/ruff/rules/

I think you want T20 rules here:

skore/skore/pyproject.toml

Lines 149 to 150 in 2eefa98

# flake8-print
"T",

Unless you also want T10 rules.

@thomass-dev thomass-dev changed the title style: Apply assorted ruff rules (RUF) style: Enforce RUF010 and RUF015 rules Oct 28, 2025
thomass-dev
thomass-dev approved these changes Oct 28, 2025
View reviewed changes
Copy link
Collaborator

@thomass-dev thomass-dev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice! I think we have enough rules for our code-style. Thanks.

@thomass-dev thomass-dev merged commit 244b994 into probabl-ai:main Oct 28, 2025
64 checks passed
@thomass-dev
Copy link
Collaborator

thomass-dev commented Oct 28, 2025

By the way, T rules are not documented: https://docs.astral.sh/ruff/rules/

I think you want T20 rules here:

skore/skore/pyproject.toml

Lines 149 to 150 in 2eefa98

# flake8-print
"T",

Unless you also want T10 rules.

We want all rules prefixed by T.

@github-actions
Copy link
Contributor

github-actions bot commented Oct 28, 2025

Coverage

Coverage Report for skore/
FileStmtsMissCoverMissing
skore/src/skore
   __init__.py230100% 
   _config.py310100% 
   exceptions.py440%4, 15, 19, 23
skore/src/skore/_sklearn
   __init__.py60100% 
   _base.py1981492%45, 58, 127, 130, 183, 186–187, 189–192, 225, 228–229
   find_ml_task.py610100% 
   types.py27196%28
skore/src/skore/_sklearn/_comparison
   __init__.py70100% 
   feature_importance_accessor.py39294%90, 109
   metrics_accessor.py178398%173, 253, 1215
   report.py1070100% 
   utils.py540100% 
skore/src/skore/_sklearn/_cross_validation
   __init__.py90100% 
   data_accessor.py45393%134, 137, 140
   feature_importance_accessor.py240100% 
   metrics_accessor.py182199%244
   report.py135199%487
skore/src/skore/_sklearn/_estimator
   __init__.py90100% 
   data_accessor.py66198%82
   feature_importance_accessor.py141298%223–224
   metrics_accessor.py356897%200, 202, 209, 300, 369, 373, 388, 423
   report.py167298%448–449
skore/src/skore/_sklearn/_plot
   __init__.py30100% 
   base.py102793%61–62, 200, 224–226, 230
   utils.py770100% 
skore/src/skore/_sklearn/_plot/data
   __init__.py20100% 
   table_report.py185199%706
skore/src/skore/_sklearn/_plot/metrics
   __init__.py60100% 
   confusion_matrix.py70494%92, 100, 122, 230
   feature_importance_display.py672168%88, 121–122, 124, 142–146, 148–155, 158–160, 162
   metrics_summary_display.py80100% 
   precision_recall_curve.py281598%455, 555, 559, 619, 751
   prediction_error.py227597%179, 186, 422, 505, 705
   roc_curve.py294897%387, 510, 515, 616, 621, 625, 694, 834
skore/src/skore/_sklearn/train_test_split
   __init__.py00100% 
   train_test_split.py580100% 
skore/src/skore/_sklearn/train_test_split/warning
   __init__.py80100% 
   high_class_imbalance_too_few_examples_warning.py19194%83
   high_class_imbalance_warning.py200100% 
   random_state_unset_warning.py100100% 
   shuffle_true_warning.py90100% 
   stratify_is_set_warning.py100100% 
   time_based_column_warning.py210100% 
   train_test_split_warning.py30100% 
skore/src/skore/_utils
   __init__.py6266%8, 13
   _accessor.py90396%34, 146, 190
   _environment.py27196%40
   _fixes.py80100% 
   _index.py50100% 
   _logger.py22481%15–17, 19
   _measure_time.py100100% 
   _parallel.py38392%23, 33, 124
   _patch.py13561%21, 23–24, 35, 37
   _progress_bar.py460100% 
   _repr_html.py80100% 
   _show_versions.py380100% 
   _testing.py550100% 
skore/src/skore/project
   __init__.py20100% 
   project.py480100% 
   summary.py75198%120
   widget.py1890100% 
TOTAL402911397% 

Tests Skipped Failures Errors Time
1079 5 💤 0 ❌ 0 🔥 4m 30s ⏱️

@github-actions
Copy link
Contributor

github-actions bot commented Oct 28, 2025

Documentation preview @ 3029401

@DimitriPapadopoulos
Copy link
Contributor Author

DimitriPapadopoulos commented Oct 28, 2025

So that would be rule sets flake8-print and flake8-debug.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@thomass-dev thomass-dev thomass-dev approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@DimitriPapadopoulos @thomass-dev