BaseObject and rework of tags system, including dynamic tags#1134
BaseObject and rework of tags system, including dynamic tags#1134
Conversation
mloning
left a comment
There was a problem hiding this comment.
Had a first look at it, haven't reviewed all of it yet.
A few additional comments, perhaps for later implementation/PR:
- in
set_tagswe should check that the passed tags are valid tags for the given estimator
I'd prefer not to, since this couples the |
Co-authored-by: Markus Löning <markus.loning@gmail.com>
Co-authored-by: Markus Löning <markus.loning@gmail.com>
|
(sorry for being pushy, but I like to believe it's for the best) |
|
from discussion: value range checks should be done by object level tests |
mloning
left a comment
There was a problem hiding this comment.
Testing is still missing, everything else looks good, only a few minor comments.
No longer! |
|
tests run locally, builds were ok before merging doc related PR, so it's fine to merge |
Rework of PR #1099 which is in turn a rework of PR #891 (by @RNKuhns), based on discussion recorded in #981 on tags.
Adds a
BaseObjectclass which adds dynamic tag functionality.BaseObjectcontainsBaseEstimatorfunctionality sansfitstate change related interfaces.The new functionality replaces the inconsistent mixture of old interface points (
all_tags,has_tag) in all downstream dependencies.post-PR, tags work like this:
_tags: dict. Individual class tags are considered to override each other as per normal nested inheritance. E.g., if a parent class and the class have different values for a class tag, the younger class' class tag value counts._tags_dynamic: dict. These are expected to be initialized in the constructor, for instance in cases where behaviour described by tags depends on components or hyper-parameters. Dynamic tags are always considered to override the class tags - e.g., if the object's class or a parent class have a static tag of the same name, the dynamic tag's value overrides any class tag's value.The new tag related methods make use of this inheritance logic are as follows.
class methods:
get_class_tags()collects all class tags and their values and returns them as adict(keys = tag names, values = tag values). It overrides values precisely as described above, in the return dictionary. This replaces the old_all_tags.get_class_tag(tag_name, tag_value_default=None)retrieves the value of an individual tag with nametag_name, and optionally allows to specify a default value if the tag is not found. The inheritance/override logic is the same as inget_class_tags.object methods:
get_tags()collects all class tags and dynamic tags and their values and returns them as adict(keys = tag names, values = tag values). It overrides values precisely as described above, in the return dictionary.get_tag(tag_name, tag_value_default=None)retrieves the value of an individual tag with nametag_name, and optionally allows to specify a default value if the tag is not found. The inheritance/override logic is the same as inget_tags.set_tags(**tag_dict)sets dynamic tags according to keys (=tag names) and values (= tag values) in the argument dictionary. It is expected to be used in constructors typically.clone_tags(estimator, tag_set=None)is a shorthand version ofset_tags, where it sets tags according to theirget_tagvalues inestimator. The tags that are mirrored/copied in this way can be restricted by specifying a list of tag names intag_set, otherwise all tags are copied over. The mirroring is by-reference and not by-value (if the type allows this), but tags are not expected to be mutated.