Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve geotop functionality #196

Merged
merged 25 commits into from
Jul 20, 2023
Merged

Improve geotop functionality #196

merged 25 commits into from
Jul 20, 2023

Conversation

dbrakenhoff
Copy link
Collaborator

@dbrakenhoff dbrakenhoff commented Jun 21, 2023

See #193 .

Introduces:

  • Restructure geotop reading to be more similar to regis (e.g. get_geotop now downloads raw data)
  • Use dask.delayed to download geotop from URL
  • Add support in caching for dask.delayed datasets
  • rewrite memory intensive operations to be more memory efficient

Warning: this PR introduces some breaking changes:

  • nlmod.read.geotop.get_geotop() now loads raw geotop data (what used to be performed by get_geotop_raw_within_extent(). Using this old method will raise a DeprecationWarning).
  • to reproduce the old get_geotop(), which converts geotop data to a layer model:
geotop_ds = nlmod.read.geotop.get_geotop(extent)  # load raw data
layer_ds = nlmod.read.geotop.to_model_layers(geotop_ds)  # convert to layer model

dbrakenhoff and others added 23 commits June 16, 2023 11:13
- use chunks and dask delayed to keep memory usage efficient
- get_geotop now gets raw data
- add deprecationwarning to get_geotop_raw_within_extent()
- now takes geotop_ds as input to convert to layers (not extent)
- do not use sortby but check diff of y dimension and flip if necessary
- use .values instead of .data to load data into memory if using dask
- undo onno changes for now w missing strat code
- use .values to deal with dask arrays
- use get_geotop
- update docstring
@rubencalje rubencalje marked this pull request as ready for review July 17, 2023 13:39
nlmod/dims/layers.py Outdated Show resolved Hide resolved
nlmod/dims/layers.py Show resolved Hide resolved
nlmod/dims/layers.py Show resolved Hide resolved
nlmod/dims/layers.py Outdated Show resolved Hide resolved
nlmod/dims/layers.py Outdated Show resolved Hide resolved
nlmod/dims/layers.py Outdated Show resolved Hide resolved
nlmod/dims/layers.py Outdated Show resolved Hide resolved
nlmod/dims/layers.py Outdated Show resolved Hide resolved
nlmod/dims/layers.py Outdated Show resolved Hide resolved
nlmod/dims/layers.py Outdated Show resolved Hide resolved
- replace Exception with more specific types
- add LayerError and MissingValueError
- fix some typos
@dbrakenhoff dbrakenhoff merged commit 787e12c into dev Jul 20, 2023
1 of 2 checks passed
@dbrakenhoff dbrakenhoff deleted the 193-geotop branch July 20, 2023 11:58
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.

3 participants