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 #372

Merged
merged 11 commits into from
Nov 30, 2023
Merged

Switch to Ruff #372

merged 11 commits into from
Nov 30, 2023

Conversation

jhkennedy
Copy link
Collaborator

@jhkennedy jhkennedy commented Nov 30, 2023

this resolves #341 by dropping autoflake, black, flake8, and isort in favor of ruff

There were some minor issues caught by ruff that I fixed as well as some differences in formatting, which IMO are improvements (this is subjective).

Notably, flake8 had a line length set at 120 while isort had it set at 88. With ruff, those are both set at 120 and there are some white-space changes, especially around unnecessarily breaking things that are shorter than 120.

I've noted any changes that are more than whitespace in the PR.

Copy link

github-actions bot commented Nov 30, 2023

Binder 👈 Launch a binder notebook on this branch for commit 7057173

I will automatically update this comment whenever this PR is modified

Binder 👈 Launch a binder notebook on this branch for commit ccd14ac

Binder 👈 Launch a binder notebook on this branch for commit 6fa4aee

Binder 👈 Launch a binder notebook on this branch for commit 9845607

Binder 👈 Launch a binder notebook on this branch for commit e5899f5

Binder 👈 Launch a binder notebook on this branch for commit 7b2d5d1

Binder 👈 Launch a binder notebook on this branch for commit 7ed8385

Copy link
Collaborator Author

@jhkennedy jhkennedy Nov 30, 2023

Choose a reason for hiding this comment

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

Looks like this workflow doesn't run here b/c it was added on a fork -- you can see it run in my fork here:
https://github.com/jhkennedy/earthdata/actions/workflows/static-analysis.yml

I added this in addition to it in the pre-commit because it will annotate the PR, which you can see in this PR:
https://github.com/ASFHyP3/hyp3-isce2/pull/156/files

@@ -51,7 +52,7 @@ def _open_files(
) -> List[fsspec.AbstractFileSystem]:
def multi_thread_open(data: tuple) -> EarthAccessFile:
urls, granule = data
if type(granule) is not str:
if not isinstance(granule, str):
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

$ ruff check earthaccess tests
earthaccess/store.py:55:12: E721 Do not compare types, use `isinstance()`

tests/integration/test_cloud_download.py Show resolved Hide resolved
@jhkennedy jhkennedy marked this pull request as ready for review November 30, 2023 07:39
pyproject.toml Outdated Show resolved Hide resolved
.github/workflows/static-analysis.yml Outdated Show resolved Hide resolved
.pre-commit-config.yaml Outdated Show resolved Hide resolved
.github/workflows/static-analysis.yml Outdated Show resolved Hide resolved
pyproject.toml Show resolved Hide resolved
pyproject.toml Show resolved Hide resolved
scripts/format.sh Outdated Show resolved Hide resolved
scripts/lint.sh Outdated Show resolved Hide resolved
jhkennedy and others added 3 commits November 30, 2023 08:01
Co-authored-by: Matt Fisher <mfisher87@gmail.com>
Co-authored-by: Matt Fisher <mfisher87@gmail.com>
MattF-NSIDC
MattF-NSIDC previously approved these changes Nov 30, 2023
Copy link

@MattF-NSIDC MattF-NSIDC left a comment

Choose a reason for hiding this comment

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

I would prefer to remove the repeated (and I think unneeded) earthaccess tests arguments to ruff. If you prefer not to do that in this PR, we can always get it in another one.

@jhkennedy
Copy link
Collaborator Author

@mfisher87 switched earthaccess tests -> .. I think this is g2g now?

Copy link

@MattF-NSIDC MattF-NSIDC left a comment

Choose a reason for hiding this comment

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

Thanks! Looks amazing :) 💯

@jhkennedy jhkennedy merged commit d6fe7f8 into nsidc:main Nov 30, 2023
7 checks passed
@jhkennedy jhkennedy deleted the ruff branch November 30, 2023 18:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Replace Flake8 with Ruff
3 participants