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

ENH: add key update_option for Well to roxar #987

Merged

Conversation

jcrivenaes
Copy link
Collaborator

@jcrivenaes jcrivenaes commented Nov 16, 2023

Closing #884

This aim with this PR is to make it possible to append selected well logs to the RMS icons. It will potentially speed up execution a lot when multiple wells with multiple logs.

The PR do also some refactoring and simplification in the _well_roxapi.py module, but no complete attempt on mypy typing, which can be a separate PR.

@jcrivenaes jcrivenaes requested review from mferrera and tnatt November 16, 2023 13:19
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.

Some suggestions are commented in but looks good to me. With future annotations I would suggest not importing the Union type and using the | notation.

Regarding your comment about *args, **kwargs on the factory functions, I think we should make an effort to address this for all the major classes.

src/xtgeo/well/_well_roxapi.py Outdated Show resolved Hide resolved
src/xtgeo/well/_well_roxapi.py Show resolved Hide resolved
src/xtgeo/well/_well_roxapi.py Outdated Show resolved Hide resolved
src/xtgeo/well/_well_roxapi.py Show resolved Hide resolved
src/xtgeo/well/_well_roxapi.py Outdated Show resolved Hide resolved
Comment on lines +161 to +163
trajectory: str = "Drilled trajectory",
realisation: int = 0,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is trajectory the trajectory name?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes

@jcrivenaes jcrivenaes force-pushed the add-possibility-append-logs-toroxar branch from 986d7ac to 7e1cc57 Compare November 19, 2023 12:54
@codecov-commenter
Copy link

Codecov Report

Attention: 1 lines in your changes are missing coverage. Please review.

Comparison is base (dc6c431) 80.28% compared to head (7e1cc57) 80.28%.

Files Patch % Lines
src/xtgeo/well/well1.py 0.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #987      +/-   ##
==========================================
- Coverage   80.28%   80.28%   -0.01%     
==========================================
  Files          92       92              
  Lines       13488    13489       +1     
  Branches     2222     2222              
==========================================
  Hits        10829    10829              
- Misses       1933     1934       +1     
  Partials      726      726              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@jcrivenaes jcrivenaes merged commit 90d16b0 into equinor:main Nov 19, 2023
27 checks passed
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