-
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: Add FLUID column and simplify column names in inplace_volumes #916
Conversation
94da8ad
to
251ff54
Compare
@@ -24,7 +25,17 @@ | |||
_logger: Final = null_logger(__name__) | |||
|
|||
|
|||
_TABLE_INDEX_COLUMNS: Final = ("ZONE", "REGION", "FACIES", "LICENCE") | |||
_TABLE_INDEX_COLUMNS: Final = ("ZONE", "REGION", "FACIES", "LICENSE") |
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.
typo in LICENSE
fixed
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.
I think this was not your typo:
fmu-dataio/src/fmu/dataio/_definitions.py
Line 73 in 61d17d4
"volumes": ["ZONE", "REGION", "FACIES", "LICENCE"], |
We probably want to change this too? Doesn't need to be in your PR
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.
👍 I'll make an issue of it, and also state that we should add FLUID
in there
251ff54
to
241edd1
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.
Very nice 👍 really like the changes in tests too.
Only concern is about the form of the final csv when output
efe0048
to
2e30e5e
Compare
2e30e5e
to
f0ba7c0
Compare
PR to add a FLUID column to the exported inplace_volumes table.
There will be more tests of the table format coming in upcoming PR #900
Closes #903