ADR Suggestion
Getting rid of the BaseObj
class
#110
damskii9992
started this conversation in
Ideas
Replies: 1 comment
-
It was discovered that the mandatory |
Beta Was this translation helpful? Give feedback.
0 replies
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
-
General
The current codebase have 3 different base classes:
BasedBase
,BaseObj
andBaseCollection
.BaseObj
andBaseCollection
inherits fromBasedBase
and adds the functionality to dynamically add attributes and methods to instances of the class during runtime.Due to confusion regarding which of
BasedBase
,BaseObj
andBaseCollection
is the actual base class to be inherited from, currently our objects have a mixture of inheriting from them. This confusion needs to be addressed by solidifying what our baseclasses are actually supposed to do.Current implementation
As mentioned, we currently have 3 "real" base classes
BasedBase
,BaseObj
andBaseCollection
:The
BasedBase
classThe
BasedBase
class inherits fromComponentSerializer
which allows the object to be serialized and deserialized (i.e. saved and loaded). It implements 5 generic attributes:global_object
,unique_name
,name
,interface
anduser_data
.Attributes
The
global_object
is a reference to the BORG classglobal_object
holding a map of all the generated easyscience objects (see #29)The
unique_name
is the objects own unique identifier in theglobal_object
The
name
is the name of the object, used when printing if there is no "display_name" or other fancy name attribute.The
interface
is an optional reference to anInterface
object. Default isNone
. Appears to be deprecated and unused.The `user_data´ is a dictionary which appears unused
Methods
On top of the regular setters and getters, the
BasedBase
class implements the following methods:Generates or re-generates bindings to the interface. Appears deprecated as we no longer use this interface template.
Similarly switches the interface to a new interface. Also appears deprecated.
Returns a list of all
Parameter
objects in the object. This functionality should not be in a base class.Returns a list of constraints of all
Parameter
objects in the object. This functionality should not be in a base class.Returns a list of "linkable"?`attributes. Not sure what this method is even supposed to do.
Returns a list of
Parameters
which are not fixed. This functionality should not be in a base class.Is apparently implemented for auto-completion only? Not sure if this is needed or not.
Defines how the object is copies. Is necessary due to the
unique_name
being an unique identifier.Makes the class pickable. Is apparently needed for serialization? Possibly not actually needed.
The
BaseObj
classThe
BaseObj
class inherits fromBasedBase
and introduces no new attributes. Instead it allow for*args
and**kwargs
to be passed to the constructor, which are then checked against existing attributes and then added as attributes to the object with theAddLoggedProp
function fromeasyscience.Utils.classTools
.To facilitate dynamically adding attributes to the object, the base class contains the following methods:
Hidden method which uses the
AddLoggedProp
to add the attribute to the Logger and to theglobal_object
.Includes all the logic to add the attribute
Handles the printing of the object. Should be in the
BasedBase
class.A wrapper to wrap the normal getter method for the dynamically added attributes. Not sure why this is needed, if at all.
A wrapper to wrap the normal setter method for the dynamically added attributes. Also not sure why this is needed.
The
BaseCollection
classThe
BaseCollection
class inherits both fromBasedBase
andMutableSequence
. Seems like it is meant to be identical toBaseObj
in that it allows for dynamically added attributes, yet is implemented as a python collection with the relevant___getitem___
,__setitem__
and__delitem__
etc. methods.This class might have some raison d´etre (reason for existing), but I am unsure of what. Custom collections certainly makes sense, but do we really need a baseclass for those?
Proposed Implementation
I propose to remove all functionality for dynamically adding attributes to objects. The only possible users I can envision are ourselves and using dynamic attributes simply makes code harder to read, write and maintain.
I also propose to remove the deprecated features from our baseclass, which is the interface methods and
Parameter
related methods.Removing dynamic attributes
Specifically I propose to entirely remove the
BaseObj
class and rename theBasedBase
class toBaseObject
. Truly making it the base class ofeasyscience
.For
BaseCollection
I propose to remove it entirely, unless it is actually needed in which case I propose to simply remove the functionality for dynamically adding attributes.Removing interface methods and attributes
It was decided in https://github.com/orgs/easyscience/discussions/29 that the
Calculator
(which is currently known asinterface
) should be constructed by a calculator factory and belong to theAnalysis
object. If other objects needs access to the calculator, they need to reference theAnalysis
class. As such I propose removing theinterface
functionality from the new base class.Additionally, if it is truly deprecated, which I believe it is or will be, I propose to remove the
InterfaceFactoryTemplate
and the entire file interface.py.Removing
Parameter
methods from the base class.While having generic functionality to go through child-objects to generate a list of
Parameters
in an object is surely a useful functionality, it only belongs to objects which are part of theSampleModel
orInstrumentModel
classes. I.e. it is not a functionality which should exist in the base class of easyscience. As such I also propose to remove those.Beta Was this translation helpful? Give feedback.
All reactions