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

Lint and format #164

Merged
merged 2 commits into from
Sep 25, 2024
Merged

Lint and format #164

merged 2 commits into from
Sep 25, 2024

Conversation

thodson-usgs
Copy link
Collaborator

Formatter did pretty well. I might tweak a couple things later, but we'll do that via the config.

We could add precommit hooks to enforce this stuff too but that can create hassle for new contributors. Let's start easy.

@ehinman
Copy link
Collaborator

ehinman commented Sep 23, 2024

Are you looking for a review, @thodson-usgs?

@thodson-usgs
Copy link
Collaborator Author

@ehinman
Good to skim though all the changes and check for instances where the formatter went too wrong. We'll either adjust the config or live with it.

df.dec_long_va.values,
df.dec_lat_va.values
)
geoms = gpd.points_from_xy(df.dec_long_va.values, df.dec_lat_va.values)
Copy link
Collaborator

Choose a reason for hiding this comment

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

This change seems a little weird, from multiple lines to one line, but not a biggie. I'm assuming this means that the line is less than 72 spaces.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

right, a little annoying but i'd live with this instance. Others might be worse such that we should try to address it now

... )
>>> # Search for features for comid: 13294314,
>>> # and data source: census2020-nhdpv2 in the upstream main
>>> search_results = dataretrieval.nldi.search(
... comid=13294314, data_source="census2020-nhdpv2", navigation_mode="UM"
... comid=13294314,
Copy link
Collaborator

Choose a reason for hiding this comment

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

This format looks good!

@@ -8,11 +8,12 @@
- implement other services like Organization, Activity, etc.

"""

Copy link
Collaborator

Choose a reason for hiding this comment

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

Seems like a random space addition?

Copy link
Collaborator

@ehinman ehinman left a 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 these changes. I don't see any glaring issues. I used chatgpt to understand the ruff specifications, which seem reasonable. What larger issues do you see? I approve, but let me know what I'm missing!

tests/iii.py Outdated Show resolved Hide resolved
tests/nadp_test.py Show resolved Hide resolved
tests/nwis_test.py Show resolved Hide resolved
assert df.index.names == [
SITENO_COL,
DATETIME_COL,
], "iv service returned incorrect index: {}".format(df.index.names)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Newline after comma?

tests/nwis_test.py Show resolved Hide resolved
tests/waterservices_test.py Show resolved Hide resolved
@thodson-usgs thodson-usgs merged commit 35de892 into DOI-USGS:main Sep 25, 2024
1 check passed
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.

2 participants