Skip to content

Network Support#586

Draft
jpalm3r wants to merge 82 commits intomainfrom
japr/network-support
Draft

Network Support#586
jpalm3r wants to merge 82 commits intomainfrom
japr/network-support

Conversation

@jpalm3r
Copy link
Collaborator

@jpalm3r jpalm3r commented Feb 19, 2026

This pull request extends the modelskill package to support network-based model results and observations. The main focus is on integrating NetworkModelResult and NodeModelResult types throughout the matching, comparison, and data handling logic, enabling seamless workflows for network geometries alongside existing point, track, and field types.

Key changes include:

Support for Network Model Results and Observations

  • Added NetworkModelResult and NodeModelResult to the public API and imports in __init__.py and model/__init__.py, and updated documentation to reflect these new types. [1] [2] [3]
  • Updated type annotations and logic in matching and comparison modules to handle NodeModelResult and NetworkModelResult alongside existing model result types. [1] [2] [3] [4] [5] [6] [7] [8]

Matching and Extraction Logic

  • Enhanced the match function and related helpers to support matching network observations and models, including correct extraction and alignment logic for network types. [1] [2] [3]

Data Handling and Conversion

  • Introduced a _drop_scalar_coords utility to consistently remove scalar coordinates (including node) from xarray datasets when converting to pandas DataFrames, ensuring clean dataframes for all geometry types. [1] [2] [3] [4] [5]
  • Modified methods for converting comparison results to observations and dataframes to handle network geometries, including new logic for the node coordinate and NodeObservation. [1] [2] [3] [4]

Attribute and Geometry Type Handling

  • Improved geometry type (gtype) inference and handling throughout the codebase to support the new network type, ensuring correct behavior and error handling for all supported geometries. [1] [2]

API and Type Annotation Consistency

  • Updated imports, type annotations, and public API exports to consistently include network-related types in all relevant modules. [1] [2] [3]

These changes collectively enable robust support for network-based model results and observations, aligning the handling of network geometries with existing modelskill workflows.

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Adds initial support for network-based model results and observations in modelskill, aiming to enable matching/comparison workflows for node-indexed time series (e.g., 1D network models) alongside existing point/track workflows.

Changes:

  • Introduces NetworkModelResult / NodeModelResult and NodeObservation / NetworkObservation types and wires them into match() and Comparer type annotations.
  • Updates TimeSeries validation and dataframe/export helpers to allow node-based coordinates (no x/y).
  • Adds a demonstration notebook for network workflows.

Reviewed changes

Copilot reviewed 14 out of 15 changed files in this pull request and generated 15 comments.

Show a summary per file
File Description
tests/test_timeseries.py Updates validation error-message assertions for new coordinate rules.
src/modelskill/types.py Adds GeometryType.NETWORK and a NetworkType alias.
src/modelskill/timeseries/_timeseries.py Allows datasets with node coordinate instead of x/y; updates x/y accessors and to_dataframe() behavior.
src/modelskill/timeseries/_point.py Adds network-node parsing and extends coordinate handling to include network coords.
src/modelskill/timeseries/_network.py New network parsing helper (currently has multiple runtime issues).
src/modelskill/timeseries/_coords.py Adds NetworkCoords.
src/modelskill/timeseries/init.py Exposes new parsing helpers.
src/modelskill/obs.py Adds NodeObservation and NetworkObservation; extends observation() factory logic.
src/modelskill/model/network.py Adds NetworkModelResult and NodeModelResult with node extraction and alignment logic.
src/modelskill/model/init.py Exports the new model result classes and updates docs.
src/modelskill/matching.py Expands NetworkObservation into NodeObservations and supports NetworkModelResult extraction.
src/modelskill/comparison/_comparison.py Broadens raw model type hints (but several point-only assumptions remain).
src/modelskill/init.py Exposes network model/observation types at package root (incomplete for NodeModelResult).
notebooks/Collection_systems_res1d.ipynb Demonstrates network matching and skill computation workflow.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copy link

Copilot AI commented Feb 20, 2026

@jpalm3r I've opened a new pull request, #587, to work on those changes. Once the pull request is ready, I'll request review from you.

@jpalm3r jpalm3r self-assigned this Feb 20, 2026
@jpalm3r jpalm3r marked this pull request as ready for review February 20, 2026 15:20
@ryan-kipawa ryan-kipawa self-requested a review February 25, 2026 08:36
ryan-kipawa
ryan-kipawa previously approved these changes Feb 25, 2026
Copy link
Collaborator

@ryan-kipawa ryan-kipawa left a comment

Choose a reason for hiding this comment

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

I skimmed the latest changes and have only two comments:

  1. For some reason, the notebook still does not show step 2 for me:
Image
  1. The PR title and description sort of implies we're done adding network support, but as I understand, there's still more PRs to come. I would suggest updating it to initial/minimal support. It's also an option to merge this into a working branch for the overall feature of network support. That way, it's still possible to make releases of modelskill without you needing to worry that each small PR is in a publishable state.

@jpalm3r
Copy link
Collaborator Author

jpalm3r commented Feb 25, 2026

I skimmed the latest changes and have only two comments:

1. For some reason, the notebook still does not show step 2 for me:

Oh, my bad. Nothing is showing in step 2 because there was nothing to render: this is legacy from a previous iteration.

Also, I like the idea of keeping this in a separate branch until is completely finished. Now that this branch is in good shape (it contains minimal working functionality). I could simply branch out and the coming PRs will merge to network-support instead of main.

@jpalm3r jpalm3r linked an issue Feb 25, 2026 that may be closed by this pull request
@ryan-kipawa ryan-kipawa dismissed their stale review February 25, 2026 09:47

We will keep this as a working feature branch.

@jpalm3r jpalm3r marked this pull request as draft February 25, 2026 10:03
@jpalm3r jpalm3r changed the title Japr/network support Network Support Feb 25, 2026
quantity: Quantity | None = None,
aux_items: list[int | str] | None = None,
attrs: dict | None = None,
) -> list[NodeObservation]: ...
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.

Integrate GNC into modelskill

5 participants