-
Notifications
You must be signed in to change notification settings - Fork 15
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
ENH: Faultroom export - add mapping of juxt.pos. to SMDA names (#724) #884
ENH: Faultroom export - add mapping of juxt.pos. to SMDA names (#724) #884
Conversation
8bb0e02
to
bd82f04
Compare
27f5595
to
2102d21
Compare
06d5791
to
cd2b240
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good 👍 just a few clarification questions from me. I'd like @tnatt to review also before we merge it
Need to get some info from Ingunn to find the best way forward regarding some of these issues |
505e04d
to
8e69a67
Compare
OK with no-ordering and warning, @tnatt ? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks much cleaner now yes 🙂 and functionality is according to our latest discussion!
My only other suggestion would be to move the method from the pydantic class to another place, to limit pydantic class scopes to primarily data storage and validation. But not that important 🙂
11bef60
to
34a0a78
Compare
I didn't really find another suitable place, so I made one myself 😊 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🚀
34a0a78
to
5c67e34
Compare
Resolves #724, and sorts the mapped names according to the given stratigraphic column.
Discussion on Slack: https://equinor.slack.com/archives/C06E3UTGTEV/p1719922723901579