ibis-project / ibis-ml

IbisML is a library for building scalable ML pipelines using Ibis.
https://ibis-project.github.io/ibis-ml/
Apache License 2.0
96 stars 13 forks source link

style(ruff): clear ruff check violations #175

Open IndexSeek opened 4 days ago

IndexSeek commented 4 days ago

I was looking through the repo and wanted to clear up the remaining outstanding ruff check violations.

Many of these are minor, and I was able to clear them by adjusting the following:

[tool.ruff.lint.per-file-ignores]
"examples/*.ipynb" = ["ERA001", "I001", "T201"]

Here are the totals before the run:

$ ruff check --statistics
17      ERA001  [*] commented-out-code
 8      T201    [*] print
 8      PT001   [*] pytest-fixture-incorrect-parentheses-style
 4      I001    [*] unsorted-imports
 3      RUF100  [*] unused-noqa
 2      E501    [ ] line-too-long
 1      B008    [ ] function-call-in-default-argument
 1      RET504  [*] unnecessary-assign
 1      PTH207  [ ] glob
 1      E721    [ ] type-comparison
 1      UP031   [*] printf-string-formatting
[*] fixable with `ruff check --fix`

I'm still getting one violation after this, not sure how to get rid of this one. I might do a bit more digging later on, if this is something we will want to do. Adding # noqa: E501 wasn't resolving it.

examples/Using IbisML and DuckDB for a Kaggle competition.ipynb:cell 22:7:89: E501 Line too long (99 > 88)
  |
5 |     Args:
6 |         file_path (str): Path to the file or regex pattern to match files.
7 |         depth (int, optional): Depth of processing. If 1 or 2, additional aggregation is performed.
  |                                                                                         ^^^^^^^^^^^ E501
8 |         is_regex (bool, optional): Whether the file_path is a regex pattern.
  |

Found 1 error.
codecov-commenter commented 3 days ago

Codecov Report

Attention: Patch coverage is 92.30769% with 1 line in your changes missing coverage. Please review.

Project coverage is 90.35%. Comparing base (c32d604) to head (ee4ac23).

Files with missing lines Patch % Lines
ibis_ml/utils/_pprint.py 0.00% 1 Missing :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #175 +/- ## ======================================= Coverage 90.35% 90.35% ======================================= Files 29 29 Lines 2083 2083 ======================================= Hits 1882 1882 Misses 201 201 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

deepyaman commented 3 days ago

Thanks! I'll try and review tomorrow or day after.

IndexSeek commented 3 days ago

Thanks! I'll try and review tomorrow or day after.

Thank you! There may be some of have been some conflicting with each other from what I found. For example, I was getting violations saying that the # noqa: SLF001 was going unused, then after removing, was getting violations running pre-commit that there were SLF001 violations.

I think these cases are squared away now.

$ ruff check
All checks passed!
$ pre-commit run --all-files
prettier.................................................................Passed
ruff.....................................................................Passed
ruff-format..............................................................Passed
Sort __all__ records.....................................................Passed
deepyaman commented 1 day ago

Actually, can you bump the Ruff version in .pre-commit-config.yaml? Because Ruff is running in CI, but it's not catching some of these things I assume because of the old version?

deepyaman commented 1 day ago

I'm still getting one violation after this, not sure how to get rid of this one. I might do a bit more digging later on, if this is something we will want to do. Adding # noqa: E501 wasn't resolving it.

examples/Using IbisML and DuckDB for a Kaggle competition.ipynb:cell 22:7:89: E501 Line too long (99 > 88)
  |
5 |     Args:
6 |         file_path (str): Path to the file or regex pattern to match files.
7 |         depth (int, optional): Depth of processing. If 1 or 2, additional aggregation is performed.
  |                                                                                         ^^^^^^^^^^^ E501
8 |         is_regex (bool, optional): Whether the file_path is a regex pattern.
  |

Found 1 error.

We can just wrap this after "is". :)

IndexSeek commented 17 hours ago

I'm still getting one violation after this, not sure how to get rid of this one. I might do a bit more digging later on, if this is something we will want to do. Adding # noqa: E501 wasn't resolving it.

examples/Using IbisML and DuckDB for a Kaggle competition.ipynb:cell 22:7:89: E501 Line too long (99 > 88)
  |
5 |     Args:
6 |         file_path (str): Path to the file or regex pattern to match files.
7 |         depth (int, optional): Depth of processing. If 1 or 2, additional aggregation is performed.
  |                                                                                         ^^^^^^^^^^^ E501
8 |         is_regex (bool, optional): Whether the file_path is a regex pattern.
  |

Found 1 error.

We can just wrap this after "is". :)

🤦 - I should have thought of this! Thank you!