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

BUG: Preserve top/base/offset information from config #773

Merged
merged 1 commit into from
Sep 9, 2024

Conversation

tnatt
Copy link
Collaborator

@tnatt tnatt commented Sep 3, 2024

Resolves #772

The top/base/offset information was dropped after roundtripping with pydantic since these fields were not defined in the GlobalConfigurationModel.
Also removed the NamedStratigraphy dataclass, as it is obsolete and can be replaced by the global_configuration.StratigraphyElement

@tnatt tnatt added the bug Something isn't working label Sep 3, 2024
@tnatt tnatt requested review from mferrera and slangeveld September 3, 2024 08:02
@tnatt tnatt self-assigned this Sep 3, 2024
@tnatt
Copy link
Collaborator Author

tnatt commented Sep 3, 2024

Added one extra commit to allow a bit more freedom in the config file, and allow top and base to be given in as strings. I suspect there might be some set-ups where this is the case (also evident from the type hints on the NamedStratigraphy).

This could later be deprecated with some information of what needs to change, currently a cryptic pydantic validation error comes telling people to use a dict input or a Layer instance... not very intuitive 🙂

Copy link
Collaborator

@mferrera mferrera left a comment

Choose a reason for hiding this comment

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

Nice catch. Just one concern here but giving the green button in case those concerns are unnecessary

src/fmu/dataio/_model/global_configuration.py Show resolved Hide resolved
Comment on lines -36 to -37
@dataclass
class NamedStratigraphy:
Copy link
Collaborator

Choose a reason for hiding this comment

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

Nice!

src/fmu/dataio/providers/objectdata/_base.py Outdated Show resolved Hide resolved
@tnatt tnatt force-pushed the strat_bug branch 2 times, most recently from aad5d23 to 6c46557 Compare September 9, 2024 11:12
@tnatt tnatt merged commit 7aff7dd into equinor:main Sep 9, 2024
13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

top, base and offset not preserved from config
2 participants