Skip to content

Development pull#1

Open
bleichro wants to merge 9 commits intomainfrom
development
Open

Development pull#1
bleichro wants to merge 9 commits intomainfrom
development

Conversation

@bleichro
Copy link
Copy Markdown
Collaborator

Ik wil graag mijn development naar main pullen.

@bleichro bleichro requested a review from Erik-Geo April 29, 2026 08:55
Erik-Geo and others added 2 commits April 29, 2026 11:17
Co-authored-by: Copilot <copilot@github.com>
Co-authored-by: Copilot <copilot@github.com>
Copy link
Copy Markdown
Collaborator

@Erik-Geo Erik-Geo left a comment

Choose a reason for hiding this comment

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

Ik heb nu een deel bekeken en leg dit vast bij je terug om mee aan de slag te gaan omdat het best al wat werk is denk ik.

Een paar dingen zoals exports bekijk ik later apart nog.

Comment thread src/submon/io/export.py
@to_geotiff.register
def _(
data: xr.DataArray,
data: xr.DataArray | xr.Dataset,
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Dit is niet correct. Deze functie is alleen de implementatie voor een xr.DataAarray, die van een Dataset staat eronder. Het principe wat je hier ziet heet function overloading en dit is hoe je het in Python implementeert: https://docs.python.org/3/library/functools.html#functools.singledispatch

Zal het eventueel nog toelichten

Comment thread src/submon/rasters.py
from dataclasses import dataclass
from pathlib import Path
from typing import TYPE_CHECKING
from unittest import result
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Waarvoor gebruik je die?

Comment thread src/submon/rasters.py
original_units=raster_l.original_units,
converted_units=raster_l.converted_units,

all_vars = sorted(set(ds_l.data_vars) | set(ds_r.data_vars))
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Dit geeft niet wat we willen. We willen de gemeenschappelijke data_vars (intersection in het Engels/programmeertermen) maar nu krijgen we alle unieke data_vars in beide xr.Datasets (een 'union').

Stel we hebben twee python sets s1 en s2:

s1 = {'a', 'b'}
s2 = { 'a', 'c'}

s1 | s2 zal nu {'a', 'b', 'c'} worden, maar we willen dat het alleen de gemeenschappelijke {'a'} wordt.

Als je deze lijn vervangt door:

set(ds_l.data_vars) & set(ds_r.data_vars)

heb je wat je wil

Comment thread src/submon/rasters.py
converted_units=raster_l.converted_units,

all_vars = sorted(set(ds_l.data_vars) | set(ds_r.data_vars))
data_vars = dict((name, ds_l.get(name, 0) + ds_r.get(name, 0)) for name in all_vars)
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Meteen als dict comprehension:

{name: ds_l.get(name, 0) + ds_r.get(name, 0) for name in all_vars}

Comment thread src/submon/rasters.py
all_vars = sorted(set(ds_l.data_vars) | set(ds_r.data_vars))
data_vars = dict((name, ds_l.get(name, 0) + ds_r.get(name, 0)) for name in all_vars)

return xr.Dataset(
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Nu worden de attrs van de linker dataset overgenomen, we moeten nog iets verzinnen zodat de attributen van de rechter bewaard blijven.

Comment thread src/submon/main.py
# read investigaqted areas
gdf = gpd.read_file(Path(config["investigated_areas"]["path"]))
gdf = gdf.to_crs(combined_stats.rio.crs)
gdf["area_m2"] = gdf.geometry.area
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Een property kan je later in je loop ook aanroepen, niet nodig om daar een kolom van te maken

Comment thread src/submon/main.py
for i, row in gdf.iterrows():
geom = row.geometry
gebied = row["Gebied"]
area = row["area_m2"]
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Op dit moment kan je geom.area gebruiken.

Dat bedoelde ik met vorige comment

Comment thread src/submon/stats.py
if clipped.isnull().all():
return (np.nan, np.nan, np.nan)

return (
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Hebben we niet alleen de mean nodig binnen een gebied? zoals de onderstaande functie suggereert ook?

Andere optie is de min en max van bodemdaling binnen een gebied meenemen als onzekerheid -> Marc vragen wat ie wil

Comment thread src/submon/main.py
gia_stats, geom, gdf.crs
)

# tectonics:
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Voorgestelde aanpak:

  1. Eerst alle datasets met statistieken clippen. Daar hoeven we geen nieuwe functie voor te schrijven want het is een simpele one-liner:

tect_stats_clipped = tect_stats.rio.clip([geom], crs=gdf.crs, drop=True, all_touched=True)

enzovoort voor gia_stats, combined_stats, mining +/- 30 jaar, total +/- 30 jaar

  1. Nu hebben we dus nog steeds al die datasets met mean, min en max arrays voor al onze bodemdalingscomponenten en combinaties daarvan, maar dan voor één gebied op dit punt in de loop. We hebben dus één functie nodig die voor elk van die datasets zoiets doet voor bijvoorbeeld tectoniek:

tect_subsidence_in_area = stats.mean_value_of_dataset_vars(tect_stats_clipped)

In deze functie loop je over alle data_vars van text_stats_clipped en neem je de mean van alle waarden in de dataarrays. Die schrijf je weg als dictionary zodat uiteindelijk tect_subsidence_in_area er als volgt uitziet met uit m'n duim gezogen waarden:

{'mean': 0.5, 'min': 0.2, 'max': 0.7}

Voordeel hiervan is dat we heel makkelijk andere statistieken kunnen implementeren mocht het nodig zijn. Bijvoorbeeld als we een dataset hebben met niet alleen mean, min en max in de toekomst, maar ook perc_10, median en perc_90 oid dan hoeven we niks aan te passen tot op dit punt in de code en komt het gewoon netjes in deze dictionary te staan. Dat is het idee van zo min mogelijk 'hardcoden' en vooruit kijken (soms verder dan nodig is, maar vak genoeg is het toch een keer nodig) + Volgens mij kunnen we alle datasets inclusief mining op deze manier hetzelfe behandelen (als je ook mijn comment van regel 55 hebt geimplementeerd).

  1. Als laatste wil je nog de textuele variant hebben voor het schrijven naar de Excel in de opmaak die Marc graag wil hebben. Hier kan je een apart utility functie voor schrijven in utils.py die een dictionary zoals bijvoorbeeld {'mean': 0.5, 'min': 0.2, 'max': 0.7} omzet naar het gewenste tekstformat

Comment thread src/submon/main.py
"Vol_Mm3_Total_next30": stats.volume(total_next30_mean, area),
}
)
df = pd.DataFrame(rows)
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Vanaf deze regel buiten de loop plaatsen! Na elk gebied update je nu het dataframe en schrijf je de Excel weg. Dus totaal wordt er meer dan 20+ overbodig een Excel weggeschreven.

Co-authored-by: Copilot <copilot@github.com>
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