Model result and observation constructors stamp `kind` into the Dataset they are handed
Nessuno ha ancora preso questa issue.
Valutazione
- Difficoltà
- 4/5
- Tempo stimato
- 3-5 giorni
- Idoneità per principianti
- 72/100
Direzione di ricerca
Start with TimeSeries.init and _validate_dataset in src/modelskill/timeseries/_timeseries.py, then inspect the five constructor sites listed in the issue and the existing vertical re-wrapping tests. Run tests/observation/test_vertical_obs.py and tests/model/test_vertical.py before adding coverage for a source observation surviving a model-result read. Done means constructors no longer mutate handed-in datasets while existing behavior remains intact.
Scritto dal modello di indicizzazione a partire dal testo della issue.
Descrizione
TimeSeries.__init__ (src/modelskill/timeseries/_timeseries.py:171) keeps the dataset it is given, by reference:
self.data = data if self._is_input_validated(data) else _validate_dataset(data)
Each constructor then marks the primary variable's kind in place. On the validated path that variable belongs to the caller, so reading an observation's data turns the observation into a model result:
o = ms.PointObservation(df, x=1.0, y=2.0, item="WL")
o.data["WL"].attrs["kind"] # 'observation'
mr = ms.PointModelResult(o.data)
o.data["WL"].attrs["kind"] # 'model' <- the observation changed
mr.data is o.data # True
Sites
The same three lines are copy-pasted at five constructors:
src/modelskill/model/point.py:69src/modelskill/model/track.py:71src/modelskill/model/vertical.py:113src/modelskill/obs.py:155src/modelskill/model/network.py— fixed onnetwork-phase-2by copying first
Observation.__init__ (src/modelskill/obs.py:152-165) goes furthest. Beyond kind it rewrites the source's dataset-level attrs, its time variable, the plot color and weight:
data.attrs = {**data.attrs, **(attrs or {})}
data["time"] = self._parse_time(data.time)
data[data_var].attrs["color"] = color
super().__init__(data=data)
self.data.attrs["weight"] = weight
_validate_dataset (_timeseries.py:117-160) also mutates its argument and returns it, so the unvalidated path is only incidentally safe: it is safe because the parse functions built a fresh object, not because anything copied.
Why it has gone unnoticed
Re-wrapping a validated dataset is rare, and the two tests that do it re-wrap same-to-same, where the kind written matches the kind already there: tests/observation/test_vertical_obs.py:184 and tests/model/test_vertical.py:170. Nothing asserts that a source observation survives being read.
DummyModelResult is the one class already safe, and it is safe because it copies — da = observation.data[observation.name].copy() (src/modelskill/model/dummy.py:65).
comparison/_comparison.py:509 is a latent instance: PointModelResult(self.data[[str(key)]], name=str(key)) stamps a variable still shared with Comparer.data, because ds[[var]] shares Variable objects. It is benign only because the kind already reads model.
Suggested fix
A shallow ds.copy() before stamping is enough — xarray rebuilds each variable's attrs dict, so no array data is copied. deep=True is not needed. Note that ds[[var]], ds.sel(...) and ds.isel(...) do not work for this: they share the Variable objects and the write goes through to the parent.
Since the stamp is duplicated five times, one helper that copies and then marks the primary variable would remove the duplication and the aliasing together. _include_attributes and _include_coords (timeseries/_point.py:150, :168) already open with ds = ds.copy(), so the discipline exists in the codebase.
- Lingua principale
- Python
- Stelle
- 56
- Fork
- 9
- Merge medio
- 1h 24m
- PR unite (30g)
- 2
Preparare l'ambiente
Avvia il container di sviluppo del progetto nel browser, con il tuo account GitHub.
- Nessun Dockerfile né file Docker Compose
- Nessun modello di pull request
- Leggi la guida per i contributori
Come iniziare
- Leggi tutta la issue e poi la guida ai contributi del progetto.
- Commenta sulla issue per dire che te ne occupi tu — evita che due persone facciano lo stesso lavoro.
- Fai un fork del repository e lavora su un branch.
- Apri una pull request che faccia riferimento al numero della issue.
Altre issue di DHI/modelskill
-
Difficoltà 2/5 1-3 ore Idoneità per principianti 76/100
DHI/modelskill#628 ·
-
Vertical: z sign convention is guessed in plots and ignored in matchingForse già presa @otzi5300 l’ha presa 7 giorni fa. Apertabug
DHI/modelskill#715 · 1 assegnatario ·
-
bug
Difficoltà 3/5 1-2 giorni Idoneità per principianti 76/100
DHI/modelskill#714 · 1 reazione ·
-
An already-validated Dataset silently discards x, y, item and the other constructor argumentsApertabug
Difficoltà 5/5 Più di una settimana Idoneità per principianti 35/100
DHI/modelskill#713 ·
-
Difficoltà 5/5 Più di una settimana Idoneità per principianti 35/100
DHI/modelskill#706 · 2 commenti ·
Tutte le issue di DHI/modelskill
Issue simili
-
Difficoltà 2/5 1-3 ore Idoneità per principianti 78/100
I maintainer di solito rispondono entro 1 giorno
-
Difficoltà 2/5 1-3 ore Idoneità per principianti 85/100
kornia/kornia#5263 · 1 commento ·
I maintainer di solito rispondono entro 1 giorno
-
approved correction metadata
Difficoltà 1/5 Meno di un'ora Idoneità per principianti 88/100
acl-org/acl-anthology#10133 · 1 commento ·
I maintainer di solito rispondono entro 1 giorno
-
Difficoltà 2/5 1-3 ore Idoneità per principianti 78/100
BasedHardware/omi#20084 ·
I maintainer di solito rispondono entro 1 giorno
-
bug needs-acceptance wg/evaluation-quality
Difficoltà 2/5 1-3 ore Idoneità per principianti 76/100
vllm-project/semantic-router#4424 ·
I maintainer di solito rispondono entro 1 giorno