Skip to content
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

Switch to Ruff #2626

Closed
wants to merge 97 commits into from
Closed

Conversation

atharva-2001
Copy link
Member

@atharva-2001 atharva-2001 commented May 24, 2024

📝 Description

Type: 🚀 feature 🚦 testing | 🎢 infrastructure

This pull request fixes all fixable errors using ruff. It also makes ruff.toml consistent with astropy.

📌 Resources

Examples, notebooks, and links to useful references.

🚦 Testing

How did you test these changes?

  • Testing pipeline
  • Other method (describe)
  • My changes can't be tested (explain why)

☑️ Checklist

  • I requested two reviewers for this pull request
  • I updated the documentation according to my changes
  • I built the documentation by applying the build_docs label

Note: If you are not allowed to perform any of these actions, ping (@) a contributor.

@tardis-bot
Copy link
Contributor

tardis-bot commented May 29, 2024

*beep* *bop*

Hi, human.

The docs workflow has succeeded ✔️

Click here to see your results.

@atharva-2001 atharva-2001 marked this pull request as ready for review May 30, 2024 12:37
@wkerzendorf
Copy link
Member

Let's please split this PR into small ones. The first should be all infrastructure changes that makes ruff compliant with astropy. Then we should probably go module by module.

@andrewfullard
Copy link
Contributor

Let's please split this PR into small ones. The first should be all infrastructure changes that makes ruff compliant with astropy. Then we should probably go module by module.

Converse argument: by merging it as one PR, we can exclude a single commit from git blame.

Keeping ANN201, has 22 occurances, TRY300 has 1, UP308 was added but we don't need
# unnecessary-iterable-allocation-for-first-element (RUF015)
# incorrect-dict-iterator (PERF102)
# useless-import-alias (PLC0414)
# needless-bool (SIM103)
# pytest-duplicate-parametrize-test-cases (PT014)
# utf8-encoding-declaration (UP009)
# yield-in-for-loop (UP028)
# extraneous-parentheses (UP034)
# if-expr-with-true-false (SIM210)
# multiple-imports-on-one-line (E401)
# unnecessary-comprehension (C416)
# unnecessary-double-cast-or-process (C414)
# unnecessary-literal-set (C405)
# dashed-underline-after-section (D407)
# new-line-after-section-name (D406)
Lowercase __path__ imported as non-lowercase TARDIS_PATH
Incorrect import of pytest; use import pytest instead
Use specific rule codes when using noqa
Assert test is a non-empty tuple, which is always True
np.recfromtxt will be removed in NumPy 2.0. Use np.genfromtxt instead.
os.path.isabs() should be replaced by Path.is_absolute()
Unnecessary elif after break statement
Trailing whitespace
Explicitly concatenated string should be implicitly concatenated
Missing from __future__ import annotations, but uses typing.Tuple
from __future__ imports must occur at the beginning of the file
Self-assignment of variable
Avoid equality comparisons to False; use if not ...: for false checks
Assert test is a non-empty tuple, which is always True
Move standard library import typing.Any into a type-checking block
Unnecessary else after raise statement
Move application import tardis.io.configuration.config_reader.ConfigurationNameSpace into a type-checking block
Do not use bare except
Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

…from tardis.io.atom_data import download_atom_data
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

5 participants