-
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
WIP: Deprecating access_ssdl argument #575
WIP: Deprecating access_ssdl argument #575
Conversation
@@ -39,6 +39,9 @@ | |||
from .datastructure.configuration import global_configuration | |||
from .providers._fmu import FmuEnv | |||
|
|||
# always show PendingDeprecationWarnings | |||
warnings.simplefilter("always", PendingDeprecationWarning) |
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.
Had to include this to make warnings actually show, not sure if this is specific for me 🤷♂️
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.
Later learned that we should use other types of warnings to make sure they are actually seen by end users.
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.
Will remove this, but keeping it for now as a TODO: We need to make sure warnings are actually seen by end users/modellers. Possibly separate PR.
Conflicts: src/fmu/dataio/_metadata.py tests/test_units/test_metadata_class.py
Just sketching at this point, but some sort of pattern is perhaps starting to emerge |
@@ -403,9 +411,10 @@ class ExportData: | |||
_inside_rms: ClassVar[bool] = False # developer only! if True pretend inside RMS | |||
|
|||
# input keys (alphabetic) | |||
access_ssdl: dict = field(default_factory=dict) | |||
access_ssdl: dict = field(default_factory=dict) # deprecated |
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.
Deprecating access_ssdl
, adding classification
and rep_include
@@ -519,6 +552,13 @@ def _show_deprecations_or_notimplemented(self) -> None: | |||
PendingDeprecationWarning, | |||
) | |||
|
|||
if self.access_ssdl: |
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.
This is the deprecation warning intended for users who still use the access_ssdl
argument
PendingDeprecationWarning, match="The 'access_ssdl' key is deprec" | ||
): | ||
ExportData(access_ssdl={"access_level": "internal", "rep_include": True}) | ||
|
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.
The patterns below are the once we want to promote going forward
ref discussion @tnatt
|
@tnatt: |
Replaced by #628 |
Ref discussion related to #540 and other issues
Proposing to deprecate the
access_ssdl
input argument in favor ofclassification
andrep_include
.This PR does:
access_ssdl
argument, introducesclassification
andrep_include
argumentsdataio.py
toglobal_configuration.py
.Might not be merged as-is, but perhaps elements or principles from it will be useful.
Purpose:
classification
argument a bit more intuitive and readableSSDL
when it is in fact not.History:
Initially, the
access.ssdl
block was specific toSSDL
and inherited from early versions of metadata when SSDL defined the FMU metadata (largely). Then, theaccess.ssdl.access_level
was used also for access control outside SSDL (i.e. Sumo). Then, we decided to deprecate it - but only go half-way there, because of multiple challenges. Essentially, it stopped after addingaccess.classification
to outgoing metadata (temporarily synchronized withaccess.ssdl.access_level
.The (milestone) goal should be to remove
access.ssdl.access_level
completely in favor ofaccess.classification
and then us this also in the config.In short, we want to go from this:
...to this:
And we want to do the same in the config, i.e. from:
to:
Complicating factors:
rep_include
parameter, so theaccess.ssdl
block will remain for now.generate_metadata()
. This makes it a bit tricky to keep things in synch, particularly when we change things as we check them. (And then check them again later.)Todo:
access_ssdl
from input argument and from config (not discontinue it)access.classification
also in the config (and test it).