KarrLab / obj_tables

Tools for creating and reusing high-quality spreadsheets
https://objtables.org
MIT License
8 stars 2 forks source link

Allow overriding of attributes when inheriting from classes in wc_kb #92

Closed paulflang closed 4 years ago

paulflang commented 4 years ago

Currently, overriding attributes managed by the ManyToX relationship manager gives for example the following error: ValueError: Attribute "obj_tables.core" of class "Evidence" inherited from "object" must be a subclass of StringAttribute because the attribute is already defined in the superclass.

jonrkarr commented 4 years ago

Please provide an example. It sounds like what you want to do is inconsistent with object oriented programming.

On Sat, Sep 28, 2019, 8:55 PM paulflang notifications@github.com wrote:

Currently overriding attributes managed by the ManyToX relationship manager gives for example the following error: ValueError: Attribute "obj_tables.core" of class "Evidence" inherited from "object" must be a subclass of StringAttribute because the attribute is already defined in the superclass.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/KarrLab/obj_tables/issues/92?email_source=notifications&email_token=AAVXMKOLEFXVFRZPO6TTRIDQL74H7A5CNFSM4I3QF5R2YY3PNVWWK3TUL52HS4DFUVEXG43VMWVGG33NNVSW45C7NFSM4HOK5PCA, or mute the thread https://github.com/notifications/unsubscribe-auth/AAVXMKNYMSW2DAVXV6KUGPDQL74H7ANCNFSM4I3QF5RQ .

paulflang commented 4 years ago

Doing the following in wc_kb/core.py branch tci_jk:

class Evidence(KnowledgeBaseObject): .... object = obj_tables.StringAttribute()

class TimeCourseEvidence(Evidence): ... object = obj_tables.ManyToOneAttribute('Observable', related_name='time_course_evidences'

Results in the error: "../obj_model/obj_tables/core.py:378: in validate_attribute_inheritance format(name, super_cls.name, attr_name, super_attr.class.name)) E ValueError: Attribute "obj_tables.core" of class "Evidence" inherited from "object" must be a subclass of StringAttribute because the attribute is already defined in the superclass"


Similarly doing

class Evidence(KnowledgeBaseObject): ... value = obj_tables.FloatAttribute()

class TimeCourseEvidence(Evidence): .... values = obj_tables.obj_math.NumpyArrayAttribute()

Results in: "TypeError: 'values_tce' is an invalid keyword argument for TimeCourseEvidence.init"

jonrkarr commented 4 years ago

We have intentionally not allowed this design pattern because it can introduce confusion for users. Generally, this design pattern should be avoided in all programming. This is codified by the Open–closed principle, one of the five major principles of object-oriented programming. This design pattern would also be inconsistent with the Principle of least astonishment.

A different design should be used that does not require changing the types of attributes in subclasses. One possibility is to use an abstract superclass rather than a concrete superclass, although some would probably argue that this design would still violate the above principles. A better design would be to (a) avoid inheritance or (b) only add attributes in subclasses.