TVB-2689: add summary_info to DataType indexes #139
Conversation
liadomide
changed the title
…n using .column_attrs.keys() and remove 'type'
popaula937
changed the title
…attributes and "id" for scientific metadata
| return ret | ||
|
|
||
| @property | ||
| def get_base_table_columns(self): |
There was a problem hiding this comment.
if you use name get_... then leave it a normal method and call with self. get_base_table_columns()
if you make it a @property, then a better name would have been def base_table_columns(self):
Particularly in this case I would use neither, but a private method (so not a property) def _get_base_table_columns(self):
| columns = self.__table__.columns.keys() | ||
| if type(self).__bases__[0] is DataType: | ||
| return columns | ||
| base_table_columns = type(self).__bases__[0].__table__.columns.keys() |
There was a problem hiding this comment.
This will go just one level up in inheritance, and not consider multiple inheritance.
Will this be enough for all our use-cases?
| columns.extend(base_table_columns) | ||
| return columns | ||
|
|
||
| def get_attribute_name(self, attr_name): |
There was a problem hiding this comment.
In general a new method is not a bad idea, but in this case, I suggest to inline the code and don't put this part in another method, because the return None convention is odd and a bit of an antipattern
After the split in multiple layers on datatypes, we no longer had "scientific Details" in a DataType overlay. We need to add something similar with the previous summary_info on DataType indexes.