Skip to content

Add tests for layer methods#450

Merged
rubencalje merged 8 commits intodevfrom
add_layers_tests
Apr 28, 2025
Merged

Add tests for layer methods#450
rubencalje merged 8 commits intodevfrom
add_layers_tests

Conversation

@rubencalje
Copy link
Collaborator

@rubencalje rubencalje commented Apr 24, 2025

This PR adds some tests for previously untested layer-methods.

It also fixes a problem with get_last_active_layer_from_idomain (which returned only zeros before) and added get_last_active_layer (like get_first_active_layer).

This PR will raise the coverage percentage to above 70 %!

image

@rubencalje rubencalje requested a review from dbrakenhoff April 25, 2025 07:59
regis2 = regis.copy(deep=True)

# botm needs to have the name "bottom'
regis2["bottom"] = regis2["botm"]
Copy link
Collaborator

Choose a reason for hiding this comment

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

Yeah, this should be fixed in the aggregate method. Let's make it a new issue.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

ok, I see you did this. Thanks!


def test_check_elevations_consistency():
regis = get_regis_horstermeer()
nlmod.layers.check_elevations_consistency(regis)
Copy link
Collaborator

Choose a reason for hiding this comment

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

perhaps we can add an inconsistent elevation to check method when the layer model isn't correct?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I added a check. The inconsistencies are reported by the logger, but it turns out you can use pytest to test logger output as well (at least locally on my computer).

gwf = nlmod.gwf.gwf(ds, sim)
nlmod.gwf.dis(ds, gwf)

nlmod.grid.interpolate_gdf_to_array(bgt, gwf, field="values", method="linear")
Copy link
Collaborator

Choose a reason for hiding this comment

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

we should try to bring some more structure to the interpolation methods? There are quite a few, and I've been working on a new one, but finding them isn't as easy as I'd hoped.

Copy link
Collaborator

Choose a reason for hiding this comment

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

This notebook gives an overview of what is available: https://nlmod.readthedocs.io/en/stable/examples/07_resampling.html

Copy link
Collaborator Author

@rubencalje rubencalje Apr 28, 2025

Choose a reason for hiding this comment

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

The interpolation methods are in https://nlmod.readthedocs.io/en/stable/examples/06_gridding_vector_data.html

I split the original notebook in a gridding_vector_data-notebook and a resample-notebook a long time ago.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Thanks for the reminders, I saw the resampling notebook but I wasn't really looking for raster ops, but somehow missed the gridding_vector data notebook. I think the new work should probably get it's own notebook, but it is essentially the opposite direction of the gridding vectors notebook (but only for points). I'll commit it now so you guys can take a look.

Copy link
Collaborator

@dbrakenhoff dbrakenhoff left a comment

Choose a reason for hiding this comment

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

Nice work! Looking good to me, I left a few comments, some can be picked up in future work.

@github-project-automation github-project-automation bot moved this from Todo to In Progress in NHFLO Apr 25, 2025
@rubencalje rubencalje merged commit 84c002b into dev Apr 28, 2025
5 checks passed
@github-project-automation github-project-automation bot moved this from In Progress to Done in NHFLO Apr 28, 2025
@rubencalje rubencalje deleted the add_layers_tests branch April 28, 2025 09:31
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