Skip to content

Fix summarize_waterdata_samples doc claims that contradict the service#889

Merged
ldecicco-USGS merged 1 commit intoDOI-USGS:developfrom
thodson-usgs:fix-summarize-samples-doc
May 5, 2026
Merged

Fix summarize_waterdata_samples doc claims that contradict the service#889
ldecicco-USGS merged 1 commit intoDOI-USGS:developfrom
thodson-usgs:fix-summarize-samples-doc

Conversation

@thodson-usgs
Copy link
Copy Markdown

@thodson-usgs thodson-usgs commented May 5, 2026

Summary

The roxygen doc for summarize_waterdata_samples contains two statements that conflict with the function's own behavior and with the underlying /samples-data/summary/{id} service.

Bug 1: comma-separated identifiers are not supported

Doc states:

Location identifiers should be separated with commas, for example:
AZ014-320821110580701, CAX01-15304600, USGS-040851385.

The function rejects vectors of length > 1:

if (length(monitoringLocationIdentifier) > 1) {
  stop("Summary service only available for one site at a time.")
}

The endpoint is path-based (/summary/{id}), so a comma-joined value would be sent as a single literal path segment, not a list.

Bug 2: bare identifiers are not auto-prefixed with USGS-

Doc states:

Location numbers without an agency prefix are assumed to have the
prefix USGS.

Verified against the live API:

$ curl -s "https://api.waterdata.usgs.gov/samples-data/summary/USGS-04183500?mimeType=text/csv" | head -2
monitoringLocationIdentifier,characteristicGroup,characteristic,characteristicUserSupplied,resultCount,activityCount,firstActivity,mostRecentActivity
USGS-04183500,Information,...,893,893,2017-01-02,2026-04-28
# 110 data rows

$ curl -s "https://api.waterdata.usgs.gov/samples-data/summary/04183500?mimeType=text/csv" | head -2
monitoringLocationIdentifier,characteristicGroup,characteristic,characteristicUserSupplied,resultCount,activities,firstActivity,mostRecentActivity
# 0 data rows; note column "activities" replaces "activityCount"

Bare identifiers return HTTP 200 with zero rows and a different response schema. No client- or server-side prefixing occurs.

Likely origin

Both statements appear to have been carried over from construct_waterdata_sample_request, whose multi-site /results/<profile> endpoint accepts list-valued query parameters. They do not apply to the single-site /summary/{id} endpoint wrapped by this function.

Change

Replace the two statements with single-site guidance and an explicit note that the agency prefix is required.

Test plan

  • Doc-only change; no behavioral risk.
  • Verified against the live USGS Water Data API on 2026-05-05.
  • Reviewer: run devtools::document() to refresh the .Rd file from this roxygen change.

Cross-reference

Identified during equivalent Python work in DOI-USGS/dataretrieval-python#262.

The doc for `summarize_waterdata_samples` carries two claims that
contradict both the function's own behavior and the live API:

1. "Location identifiers should be separated with commas, for example:
   AZ014-..., CAX01-..., USGS-..." — but the function itself rejects
   multi-ID input with `if (length > 1) stop("Summary service only
   available for one site at a time.")`. The summary endpoint is
   path-based (`/summary/{id}`), so a comma in the input would just
   become part of a single weird path segment.

2. "Location numbers without an agency prefix are assumed to have the
   prefix USGS." — verified against the live API: a bare ID (e.g.
   `04183500`) returns HTTP 200 with zero data rows and a different
   column shape than the prefixed version (`activities` vs
   `activityCount`). The auto-prefix behavior is not what the service
   does.

Both claims appear to have been copy-pasted from
`construct_waterdata_sample_request`'s doc, where they may apply to
the multi-site `/results/<profile>` endpoint but do not apply to the
single-site `/summary/{id}` endpoint that this function wraps.

Replaces the two incorrect sentences with accurate single-site
guidance and notes the agency-prefix requirement.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@ldecicco-USGS ldecicco-USGS marked this pull request as ready for review May 5, 2026 21:09
@ldecicco-USGS ldecicco-USGS merged commit 1779879 into DOI-USGS:develop May 5, 2026
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