Skip to content

Commit

Permalink
incorprorate review comments
Browse files Browse the repository at this point in the history
  • Loading branch information
perrygreenfield committed Sep 28, 2023
1 parent 9e478f6 commit e865c7b
Show file tree
Hide file tree
Showing 4 changed files with 9 additions and 49 deletions.
2 changes: 2 additions & 0 deletions CHANGES.rst
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@
===================
- Fix Node assignment issues. [#275]

- Fix newly required units from rand [#256]

- Update minimum version of astropy to 5.3.0 in order to fix a bug due to a breaking
change in astropy. [#258]

Expand Down
8 changes: 1 addition & 7 deletions docs/roman_datamodels/datamodels/using_datamodels.rst
Original file line number Diff line number Diff line change
Expand Up @@ -101,17 +101,11 @@ page::
There are a couple subtlties with regard to changing values in a datamodel.
If you assign dicts or lists to attributes, it will map these into the
corresponding DNode or LNode subclasses. In such uses, the assigned values
will be immediately be checked by validating against the defining schemas.
will be immediately checked by validating against the defining schemas.
When the value being assigned fails to pass that validation, an exception
will occur. This is generally a good thing, particularly if you are changing
values interactively

On the other hand, if one assigns to the attribute a subclass of DNode
or LNode, no immediate validation is done (for current implenentation
reasons). If the wrong kind of DNode or LNode is assigned to the
attribute that doesn't match the expected tag for that attribute, the error
will only be raised when trying to write the file to disk.

If you are getting validation errors consult the corresponding schema in
``rad`` to se what is allowed. If you think the schema is wrong, or you
continue to have issues, please contact the Roman team for help.
Expand Down
41 changes: 0 additions & 41 deletions src/roman_datamodels/stnode/_converters.py
Original file line number Diff line number Diff line change
Expand Up @@ -114,50 +114,9 @@ def from_yaml_tree(self, node, tag, ctx):

return SCALAR_NODE_CLASSES_BY_TAG[tag](node)

# The following converters are intended allow the current node behavior of
# accessing node attributes that are simple lists and dicts and returning
# them as LNode and DNode instances so that the attribute machinery works
# (i.e., that it is possible to access dict members as attributes rather
# than requiring the usual index syntax). This presents a problem with
# assigning such instances where simple lists and dicts are expected (and
# thus failing validation). This is solved by providing converters for
# DNode and LNode that turn instances back to dicts and lists on serialization
# so that they pass validation. Only conversion is needed for serialization,
# and not for deserializing since there are no tags that can be used for
# that.

class DNodeConverter(Converter):
tags = []
types = ["roman_datamodels.stnode._node.DNode"]

def select_tag(self, obj, tags, ctx):
# return tags[0]
return None

def to_yaml_tree(self, obj, tag, ctx):
return obj._data

def from_yaml_tree(self, node, tag, ctx):
raise NotImplementedError()


class LNodeConverter(Converter):
tags =[]
types = ["roman_datamodels.stnode._node.LNode"]

def select_tag(self, obj, tags, ctx):
# return tags[0]
return None

def to_yaml_tree(self, obj, tag, ctx):
return obj.data

def from_yaml_tree(self, node, tag, ctx):
raise NotImplementedError()

# Create the ASDF extension for the STNode classes.
NODE_EXTENSIONS = [
ManifestExtension.from_uri("asdf://stsci.edu/datamodels/roman/manifests/datamodels-1.0",
converters=NODE_CONVERTERS.values()),
ManifestExtension(None, converters=[DNodeConverter(), LNodeConverter()]),
]
7 changes: 6 additions & 1 deletion src/roman_datamodels/stnode/_node.py
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,10 @@ def _check_value(value, schema, validator_context):


def _validate(attr, instance, schema, ctx):
if type(instance) == DNode:
instance = instance._data

Check warning on line 64 in src/roman_datamodels/stnode/_node.py

View check run for this annotation

Codecov / codecov/patch

src/roman_datamodels/stnode/_node.py#L64

Added line #L64 was not covered by tests
elif type(instance) == LNode:
instance = instance.data
tagged_tree = yamlutil.custom_tree_to_tagged_tree(instance, ctx)
return _value_change(attr, tagged_tree, schema, False, will_strict_validate(), ctx)

Expand Down Expand Up @@ -148,7 +152,8 @@ def __setattr__(self, key, value):
if key in self._data:
if will_validate():
schema = _get_schema_for_property(self._schema(), key)
schema == {} or _validate(key, value, schema, self.ctx)
if schema:
_validate(key, value, schema, self.ctx)
self._data[key] = value
else:
raise AttributeError(f"No such attribute ({key}) found in node")
Expand Down

0 comments on commit e865c7b

Please sign in to comment.