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

Fix remove layer #307

Merged
merged 1 commit into from
Dec 19, 2023
Merged

Fix remove layer #307

merged 1 commit into from
Dec 19, 2023

Conversation

rubencalje
Copy link
Collaborator

When the first layer is removed, the top needs to be set on the botm of this layer, not on the botm of the second layer.

@rubencalje
Copy link
Collaborator Author

Tests fail unrelated to thsi pr

@rubencalje rubencalje marked this pull request as ready for review December 13, 2023 11:17
# lower the top to the second layer
ds["top"] = ds["botm"].loc[layers[1]]
# lower the top to the botm of the first layer
ds["top"] = ds["botm"].loc[layers[0]]
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think the docstring needs to be changed. It says now: "Removes a layer from a Dataset, without changing elevations of other layers". The last part is (and was already) incorrect.

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 think the docstring is (still) correct. The top and botm of other layers are not changed. Only when the top layer is removed, the top of the new top layer (that used to be the second layer) is set to the variable top. The top of the second layer is not changed, only the way it is represented in ds changes.

@rubencalje rubencalje merged commit c712010 into dev Dec 19, 2023
2 of 3 checks passed
@rubencalje rubencalje deleted the fix_remove_layer branch July 5, 2024 13:08
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