Skip to content

Require explicit item and z_item on vertical classes#659

Open
ecomodeller wants to merge 2 commits into
mainfrom
require-vertical-item-zitem
Open

Require explicit item and z_item on vertical classes#659
ecomodeller wants to merge 2 commits into
mainfrom
require-vertical-item-zitem

Conversation

@ecomodeller
Copy link
Copy Markdown
Member

@ecomodeller ecomodeller commented May 18, 2026

Drops the defaults z_item=0 and item=None-with-2-column-auto-pick on VerticalObservation and VerticalModelResult. Both arguments become required.

Raise ValueError on duplicate (time, z) entries instead of silently
deduplicating. Vertical profiles must have a unique depth per timestamp;
duplicates indicate a data-quality issue that the caller should resolve
before construction, not a sampling artifact the library should paper over.
…elResult

Drops the positional defaults (`z_item=0`, `item=None` with 2-column auto-pick).
A vertical profile's depth axis and value column are the defining inputs;
defaulting them to "column 0" / "column 1" is a position-based assumption in
an API that should be name-based. The same flaw exists on TrackObservation
and TrackModelResult but those have real users and will get a separate
deprecation PR; vertical is alpha and can break cleanly.

Internal transforms (`sel`, `trim`, etc.) reach the constructors via the
base `_create_new_instance` which calls `self.__class__(data)` with no
kwargs. Two overrides reconstruct `item` from `self.name` and use
`z_item="z"` (always renamed by the parser), preserving the prevalidated-
dataset path without leaking sentinel defaults into the public signature.

Two other internal call sites that bypassed `_create_new_instance`
(`comparison/_comparison.py` and `comparison/_vertical_comparison.py`)
now go through it for consistency.

Also fixes a docstring typo (`mikeio.Dfs0, mikeio.Dfs0` -> `mikeio.Dfs0,
mikeio.Dataset`) and switches both data-arg docstrings from type-listing
to semantic description.
@ecomodeller ecomodeller changed the base branch from remove-vertical-keep-duplicates to main May 18, 2026 21:28
@ecomodeller ecomodeller requested a review from Copilot May 18, 2026 21:30
@ecomodeller ecomodeller reopened this May 18, 2026
Copy link
Copy Markdown
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.

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

@ecomodeller ecomodeller requested a review from otzi5300 May 19, 2026 06:07
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