Skip to content

Conversation

AlexanderJuestel
Copy link
Contributor

@AlexanderJuestel AlexanderJuestel commented Sep 30, 2025

Description

This PR maps the formation according to its ID to each surface point and orientation since this information is currently not available. Further, the id_to_name function was fixed to actually take an ID from the surface points DataFrame and return the corresponding formation name. Previously, the index was taken, not the ID of the formation. A corresponding function in structural_frame.py was adapted,

Relates to #1063

image

Checklist

  • My code uses type hinting for function and method arguments and return values.
  • I have created tests which cover my code.
  • The test code either 1. demonstrates at least one valuable use case (e.g. integration tests)
    or 2. verifies that outputs are as expected for given inputs (e.g. unit tests).
  • New tests pass locally with my changes.

@graphite-app graphite-app bot added the gempy 3 Will come with the next major update label Sep 30, 2025
Copy link

graphite-app bot commented Sep 30, 2025

Graphite Automations

"Add gempy label" took an action on this PR • (09/30/25)

1 label was added to this PR based on Miguel de la Varga's automation.

Copy link
Member

Leguark commented Oct 7, 2025

Hey Alex, thanks a lot for the PR! Adding an extra column here might look convenient, but it would come with performance and memory costs and risk duplicating data. That could lead to mismatches between IDs and formations — similar to the stability problems we had in GemPy 2.

If the goal is just to make the DataFrame look nicer in notebooks, I’d suggest adding the column dynamically in the df property using the mapping field and IDs. This approach keeps the GemPy model definition clean and unaffected.

@Leguark Leguark marked this pull request as draft October 7, 2025 11:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
gempy 3 Will come with the next major update
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants