Skip to content

Commit

Permalink
Fix an inconsistency in the MIVOT instance key generation ("." in keys).
Browse files Browse the repository at this point in the history
  • Loading branch information
lmichel authored and bsipocz committed May 7, 2024
1 parent 4875b5b commit 03b26e7
Show file tree
Hide file tree
Showing 2 changed files with 18 additions and 12 deletions.
12 changes: 6 additions & 6 deletions pyvo/mivot/tests/test_user_api.py
Original file line number Diff line number Diff line change
Expand Up @@ -276,24 +276,24 @@ def test_with_full_dict(path_to_full_mapped_votable):
"EpochPositionErrors_parallax": {
"dmrole": "parallax",
"dmtype": "PropertyError1D",
"PropertyError1D.sigma": {"value": 0.06909999996423721, "unit": "mas"},
"sigma": {"value": 0.06909999996423721, "unit": "mas"},
},
"EpochPositionErrors_radialVelocity": {
"dmrole": "radialVelocity",
"dmtype": "PropertyError1D",
"PropertyError1D.sigma": {"value": None, "unit": "km/s"},
"sigma": {"value": None, "unit": "km/s"},
},
"EpochPositionErrors_position": {
"dmrole": "position",
"dmtype": "ErrorMatrix",
"ErrorMatrix.sigma1": {"value": 0.0511, "unit": "mas"},
"ErrorMatrix.sigma2": {"value": 0.0477, "unit": "mas"},
"sigma1": {"value": 0.0511, "unit": "mas"},
"sigma2": {"value": 0.0477, "unit": "mas"},
},
"EpochPositionErrors_properMotion": {
"dmrole": "properMotion",
"dmtype": "ErrorMatrix",
"ErrorMatrix.sigma1": {"value": 0.06400000303983688, "unit": "mas/yr"},
"ErrorMatrix.sigma2": {"value": 0.06700000166893005, "unit": "mas/yr"},
"sigma1": {"value": 0.06400000303983688, "unit": "mas/yr"},
"sigma2": {"value": 0.06700000166893005, "unit": "mas/yr"},
},
},
"EpochPosition_correlations": {
Expand Down
18 changes: 12 additions & 6 deletions pyvo/mivot/viewer/mivot_instance.py
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ class MivotInstance:
def __init__(self, **instance_dict):
"""
Constructor of the MIVOT class.
Parameters
----------
kwargs (dict): Dictionary of the XML object.
Expand Down Expand Up @@ -68,6 +69,7 @@ def _create_class(self, **kwargs):
For the unit of the ATTRIBUTE, we add the Astropy unit or the Astropy time equivalence by comparing
the value of the unit with values in time.TIME_FORMATS.keys() which is the list of time formats.
We do the same with the unit_mapping dictionary, which is the list of Astropy units.
Parameters
----------
kwargs (dict): Dictionary of the XML object.
Expand All @@ -79,7 +81,7 @@ def _create_class(self, **kwargs):
getattr(self, self._remove_model_name(key)).append(MivotInstance(**item))
elif isinstance(value, dict): # INSTANCE
if not self._is_leaf(**value):
setattr(self, self._remove_model_name(key, True), MivotInstance(**value))
setattr(self, self._remove_model_name(key, role_instance=True), MivotInstance(**value))
if self._is_leaf(**value):
setattr(self, self._remove_model_name(key), MivotInstance(**value))
else: # ATTRIBUTE
Expand All @@ -100,6 +102,7 @@ def update(self, row, ref=None):
"""
Update the MIVOT class with the new data row.
For each leaf of the MIVOT class, we update the value with the new data row.
Parameters
----------
row (astropy.table.row.Row): The new data row.
Expand All @@ -116,17 +119,18 @@ def update(self, row, ref=None):
if 'value' in vars(value):
value.update(row=row, ref=getattr(value, 'ref'))
else:
if key == 'value':
if ref is not None and ref != 'null':
setattr(self, self._remove_model_name(key),
MivotUtils.cast_type_value(row[ref], getattr(self, 'dmtype')))
if key == 'value' and ref is not None and ref != 'null':
setattr(self, self._remove_model_name(key),
MivotUtils.cast_type_value(row[ref], getattr(self, 'dmtype')))

@staticmethod
def _remove_model_name(value, role_instance=False):
"""
Remove the model name before each colon ":" as well as the type of the object before each point ".".
If it is an INSTANCE of INSTANCEs, the dmrole represented as the key needs to keep his type object.
In this case (`role_instance=True`), we just replace the point "." With an underscore "_".
- if role_instance: a:b.c -> b_c else c
Parameters
----------
value (str): The string to process.
Expand All @@ -138,7 +142,7 @@ def _remove_model_name(value, role_instance=False):
index_underscore = value.find(":")
if index_underscore != -1:
# Then we find the object type before the point
next_index_underscore = value.find(".", index_underscore + 1)
next_index_underscore = value.rfind(".")
if next_index_underscore != -1 and not role_instance:
value_after_underscore = value[next_index_underscore + 1:]
else:
Expand All @@ -152,6 +156,7 @@ def _remove_model_name(value, role_instance=False):
def _is_leaf(self, **kwargs):
"""
Check if the dictionary is an ATTRIBUTE.
Parameters
----------
**kwargs (dict): The dictionary to check.
Expand All @@ -169,6 +174,7 @@ def _get_class_dict(self, obj, classkey=None, slim=False):
"""
Recursively displays a serializable dictionary.
This function is only used for debugging purposes.
Parameters
----------
obj (dict or object): The dictionary or object to display.
Expand Down

0 comments on commit 03b26e7

Please sign in to comment.