Add request partitioning to waterdata_client.

[jarq6c]

This PR adds location-based request partitioning to the waterdata_client and brings the client into beta with version 0.9.0b0. The primary client interfaces are stable and no new features will be added until after the first production/stable release (1.0.0). A roadmap for v1.0.0 is forthcoming.

Changes

• scripts/build_clients.py Added new StringList type for monitoring_location_id arguments.
• scripts/build_request_models.py Added new StringList type for monitoring_location_id arguments.
• clients/*.py Regenerated all client modules.
• request_models/*.py Regenerated all request model modules.
• templates/clients.py.j2 Includes new Jinja2 boolean namespace variable called location_id. This triggers the addition of the new StringList type and additional logic for handling monitoring_location_id when generating queries passed to get_all. Also accommodates changes to request_models.py.j2.
• templates/request_models.py.j2 Includes new Jinja2 boolean namespace variable called location_id. This triggers the addition of the new StringList type and additional logic for handling monitoring_location_id when generating queries passed to get_all. The additional logic uses a new method called group_string_iterable to chunk a list of string into comma-delimited lists compatible with WaterData APIs.
• _version.py Bumped to 0.9.0b0
• base_client.py Adds new chunk_size attribute to BaseClient.
• client_config.py Adds new chunk_size attribute to package-wide SETTINGS and new environment variable HYDROTOOLS_CHUNK_SIZE.
• request_paritioning.py New module with three new methods. batched is a python 3.11 friendly implementation of the itertools.batched method added in python 3.12. The python 3.12 implementation is written in C and theoretically faster, so the module imports that if available. group_string_iterable takes a str | Iterable[str] and returns a list of str where the elements are chunked comma-delimited lists of items from the original Iterable. validate_sequence_argument is a Pydantic validation function used with the new StringList type that makes monitoring_location_id a list[str].

Testing

• tests/test_clients.py Added tests that clients correctly pull default chunk_size from package-wide SETTINGS, allowing override at time of call. Checks for correct number of unique URLs with correctly added list of locations.
• tests/test_iterable_conversion.py New tests for Pydantic models that use the new partitioning logic. Includes defaults, overrides, query generation, and correctly raised pydantic.ValidationError.
• tests/test_partitioning.py New tests of underlying stand-alone partitioning methods.

Checklist

• PR has an informative and human-readable title
• PR is well outlined and documented. See #12 for an example
• Changes are limited to a single goal (no scope creep)
• Code can be automatically merged (no conflicts)
• Code follows project standards (see CONTRIBUTING.md)
• Passes all existing automated tests
• Any change in functionality is tested
• New functions are documented (with a description, list of inputs, and expected output) using numpy docstring formatting
• Reviewers requested with the Reviewers tool ➡️

[christophertubbs]

I identified an anti-pattern and some subjective nit-picks. It looks good enough to ship, so there's nothing blocking it.

Consider some of the comments in further development. Consider opening the pathway (without necessarily implementing it) to partition by other fields or multiple fields.

[christophertubbs]

q is not a sufficient variable name, especially in our domain ("What is discharge doing here?"). Add a ticket for hunting down and replacing these types of variable definitions. Something like ^\s*([a-zA-Z]\s*(:([^=])*)?=.+)$ may help you find everything in the search bar in vscode or pycharm.

[christophertubbs]

Single character variable names don't necessarily need to follow the full naming rules so it doesn't need to follow the rule stated above. Nest a deeper comprehension, however, and naming rules come back into play.

[christophertubbs]

deepcopy is a bit over the top for a shallow dict. The following may work "better" for partitioning:

for group in grouped_list:
    query = {
        **{
            key: value
            for key, value in base_query.items()
            if value is not None
        },
        "monitoring_location_id": group
    }
    queries.append(query)

I wouldn't further compress the immediately prior logic since it'll add unnecessary cognitive overhead.

It may be worth putting the following line above the partitioning logic:

base_query = {k: v for k, v in query.items() if v is not None}

and the following at the end:

return [base_query]

you may also want to consider invalid empty values in the filter for None values. A base_query value of "properties": {} may cause some heart ache either now or if there is a change later down the line with unfortunate and unexpected side effects.

Don't consider this a MUST FIX - some of it is a subjective preference and what is provided seems like it'll work. This all may be worth a low-hanging-fruit ticket.

[christophertubbs]

Another subjective comment: consider reformatting as:

batch = tuple(islice(iterator, n))
while batch:
    if strict and len(batch) !=n:
        raise ValueError("batched(): incomplete batch")
    yield batch
    batch = tuple(islice(iterator, n))

or

while True:
    batch = tuple(islice(iterator, n))
    if not batch:
        break
    if strict and len(batch) != n:
        raise ValueError("batched(): incomplete batch")
    yield batch

While Python added := to make it easier to do what you're doing, you're also performing a mutation in your conditional check. It's a classic no-no (outside of a for-loop). The typical names for that anti-pattern are hidden side effect or hidden mutation.

[jarq6c]

I identified an anti-pattern and some subjective nit-picks. It looks good enough to ship, so there's nothing blocking it.

Consider some of the comments in further development. Consider opening the pathway (without necessarily implementing it) to partition by other fields or multiple fields.

@christophertubbs Thanks for the review. I'll add "assessment and mitigation of anti-patterns" to the robustness ticket.

I was originally going to attempt a more general partitioning solution, however, it was a bit out of scope for this particular development cycle. I figured getting a site-based partitioning scheme was a good place to start. I do think the necessary elements are present to expand partitioning to other arguments (query intervention, request model expansion, and a chunk parameter).