Skip to content

Conversation

@ehinman
Copy link
Collaborator

@ehinman ehinman commented Dec 10, 2025

...move probably-useless-to-user database ids for specific endpoints to the end of the dataframe, so they're less likely to impede review of the data returned.

Also added a couple tests for these changes. Note that the time component is sorted in ascending order, so earlier results show up first.

Closes #202 and is a slightly softer solution to #201, since users can request columns in the properties argument, anyway.

@ehinman ehinman requested a review from thodson-usgs December 10, 2025 22:14
Copy link
Collaborator

@thodson-usgs thodson-usgs left a comment

Choose a reason for hiding this comment

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

looks good,

# of the output_id (e.g. "monitoring_locations_id"), then rename
# "id" to plural. This is pretty niche.
else:
plural = output_id.replace("_id", "s_id")
Copy link
Collaborator

Choose a reason for hiding this comment

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

this line confuses me, but the code appears to work as it should

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

We can revisit, because this might not be necessary. I'll make an issue. Basically, every service returns a straight up "id" column with the data, which is actually different across services. So the package adds the service name to the beginning of the "id" column, e.g. "monitoring_location_id", "daily_id", etc. This part of the function accounts for whether someone enters just "id" into their properties argument, or enters "monitoring_locationS_id" (maybe they notice that pattern that it's service + id, and the sites service is called "monitoring-locationS"). If they enter "id", then the resulting dataframe will have the "monitoring_location_id" column name. But if they enter "monitoring_locations_id" (straight up service name, "monitoring locations", plus "id"), then it will return the column name "monitoring_locations_id". I kinda doubt this will be leveraged at all, and adds confusion.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@ehinman ehinman merged commit 98f4ddd into DOI-USGS:main Dec 11, 2025
7 checks 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.

waterdata module - order returned data in a logical way

2 participants