Hacktoberfest 2026: le issue che i maintainer hanno segnato per ottobre, aperte e adatte ai principianti. Sfoglia le issue Hacktoberfest

An already-validated Dataset silently discards x, y, item and the other constructor arguments

Aperta
  • Qualcosa non va?
#713 0 commenti 0 reazioni 0 assegnatari Vedi su GitHub

Nessuno ha ancora preso questa issue.

Valutazione

Difficoltà
5/5
Tempo stimato
Più di una settimana
Idoneità per principianti
35/100
Tipo di issue
Bug
Chiarezza
Abbastanza chiara
Stato di attività
Attiva
Stack tecnologico
python
Ambito
backend, data

Direzione di ricerca

Start with _is_input_validated in src/modelskill/timeseries/_timeseries.py and trace the six observation and model-result constructors listed in the issue. Then inspect VerticalAccessor._agg in src/modelskill/comparison/_vertical_comparison.py and the network-class behavior; done means the validated-input semantics are decided per argument and the vertical aggregation path remains correct.

Scritto dal modello di indicizzazione a partire dal testo della issue.

Descrizione

bug

Every observation and model result constructor skips parsing when the input is already a modelskill dataset:

if not self._is_input_validated(data):
    data = _parse_xyz_point_input(data, name=name, item=item, quantity=quantity,
                                  aux_items=aux_items, x=x, y=y, z=z)

_is_input_validated (src/modelskill/timeseries/_timeseries.py:182) keys on one dataset-level attribute:

return isinstance(data, xr.Dataset) and "modelskill_version" in data.attrs

Parsing is where x, y, z, item, name, quantity and aux_items are applied, so on that path every one of them is discarded without a word:

o = ms.PointObservation(df, x=1.0, y=2.0, item="WL")

o2 = ms.PointObservation(o.data, x=999.0, y=999.0)
o2.x, o2.y        # (1.0, 2.0)   <- the arguments were dropped

mr = ms.PointModelResult(o.data, name="renamed")
mr.name           # 'WL'         <- likewise

Sites

  • src/modelskill/obs.py:247 PointObservation
  • src/modelskill/obs.py:366 TrackObservation
  • src/modelskill/obs.py:473 VerticalObservation
  • src/modelskill/model/point.py:54 PointModelResult
  • src/modelskill/model/track.py:57 TrackModelResult
  • src/modelskill/model/vertical.py:100 VerticalModelResult

NodeObservation (obs.py:588) and ReachObservation (obs.py:794) did the same until network-phase-2, where they were changed to raise when the location named disagrees with the one the data carries.

Why the rest cannot simply follow

One internal caller depends on the drop. VerticalAccessor._agg (src/modelskill/comparison/_vertical_comparison.py:279) builds a PointModelResult with an x/y that legitimately differs from the data's:

raw_mod_data[mod_name] = PointModelResult(raw, x=cmp.x, y=cmp.y, quantity=cmp.quantity)

cmp.x is the observation's position, carried into the matched dataset by matching.py:428, while raw derives from cmp.raw_mod_data[...].data, which carries the dfsu element centre (src/modelskill/model/dfsu.py:233-234). raw comes from getattr(r_grouped, agg_func)(), and groupby("time").mean() preserves modelskill_version, so it takes the validated path and the arguments are dropped today.

So cmp.vertical.mean() currently works because of this bug. Deciding what it should do is a prerequisite for fixing the general case.

Suggested fix

Decide per argument what the validated path means, rather than applying one rule to all of them:

  • A location argument that contradicts the data is a mistake — raise, as the network classes now do.
  • name and quantity can be applied after the fact; TimeSeries already has setters for both (_timeseries.py:195, :205).
  • item and aux_items cannot be honoured at all once parsing is skipped, so they should raise.

Settle VerticalAccessor._agg first: either pass the coordinates through a path that honours them, or stop passing them.

Lingua principale
Python
Stelle
56
Fork
9
Merge medio
1h 24m
PR unite (30g)
2

Preparare l'ambiente

Apri in Codespaces

Avvia il container di sviluppo del progetto nel browser, con il tuo account GitHub.

Come iniziare

  1. Leggi tutta la issue e poi la guida ai contributi del progetto.
  2. Commenta sulla issue per dire che te ne occupi tu — evita che due persone facciano lo stesso lavoro.
  3. Fai un fork del repository e lavora su un branch.
  4. Apri una pull request che faccia riferimento al numero della issue.

Altre issue di DHI/modelskill

Tutte le issue di DHI/modelskill

Issue simili

Altre issue su Python

Ricevi le nuove issue nella tua casella

Un breve riepilogo di issue GitHub adatte ai principianti.