SYNPY-795 & SYNPY-797 #577
Conversation
This reverts commit b5b77e1.
synapseclient/table.py
Outdated
| @@ -861,6 +864,10 @@ def __init__(self, id=None, columnType=None, name=None): | |||
| def from_column(cls, column): | |||
| return cls(column.get('id', None), column.get('columnType', None), column.get('name', None)) | |||
|
|
|||
| @classmethod | |||
| def from_data(cls, data): | |||
There was a problem hiding this comment.
from_dict would make it more clear that the expected arguments are dicts
synapseclient/table.py
Outdated
| @@ -1075,6 +1085,10 @@ def __init__(self, values, rowId=None, versionNumber=None, etag=None): | |||
| if etag is not None: | |||
| self.etag = etag | |||
|
|
|||
| @classmethod | |||
| def from_data(cls, data): | |||
There was a problem hiding this comment.
Would prefer **kwargs to be present in the constructor for this method because Rows are sent back and forth between the client and backend. kwargs would need to be documented to indicate that they are used only for unexpected fields introduced in newer versions of the backend. Using kwargs with self.update(kwargs) ensures that all of the fields are saved within the scope of the class instance's constructor. The alternative is to do:
@classmethod
def from_data(cls, data):
copy_dict = dict(data)
inst = cls(copy_dict .pop('values'), copy_dict .pop('rowId', None), copy_dict .pop('versionNumber', None), copy_dict .pop('etag', None))
inst.update(copy_data)
return inst
This is not only messier stylistically but it also tags on unexpected fields (from newer versions of the JSON object) into an already partially instantiated object. It would not be clear to someone just reading the constructor that unexpected fields are allowed/exist.
No description provided.