-
Notifications
You must be signed in to change notification settings - Fork 3
New labelling system #115
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: v4.x
Are you sure you want to change the base?
New labelling system #115
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,41 @@ | ||
| from typing import Any, Iterable, Mapping | ||
| from collections import UserDict | ||
| from .util_types import StaticTypedValueDict | ||
|
|
||
|
|
||
| class Labels(StaticTypedValueDict): | ||
| """Subclass of `StaticTypedValueDict` that only allows for string values.""" | ||
|
|
||
| def __init__(self, iterable: Iterable) -> None: | ||
| super().__init__(iterable, str) | ||
|
|
||
|
|
||
| class Labelling: | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can you do the docstring please?
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What is a |
||
| """Labelling can hold only `Labels` where all their keys are a subset of the nodes of `LineageTree`.""" | ||
|
|
||
| default_dict = {} | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. SHouldn't it be |
||
|
|
||
| def __init__(self, nodes) -> None: | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What is |
||
| if isinstance(nodes, Iterable) and not isinstance(nodes, str): | ||
| self.__dict__["_nodes"] = set(nodes) | ||
|
Comment on lines
+19
to
+20
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. why are you doing self.__dict__["_nodes"] = set(nodes)rather than self._nodes = set(nodes)
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Because the setattr is custom |
||
| else: | ||
| raise ValueError( | ||
| f"Labelling expects an Iterable, not {type(nodes)}" | ||
| ) | ||
|
|
||
| def __setattr__(self, name: str, value: dict | Labels) -> None: | ||
| l = Labels(value) | ||
| if not set(l).issubset(self._nodes): | ||
| raise ValueError( | ||
| "All ids in the labelset should correspond to ids in the LineageTree object." | ||
| ) | ||
|
Comment on lines
+28
to
+31
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Does it create errors if you have keys that are not in the set of nodes? |
||
| if ( | ||
| not self.default_dict | ||
| ): # if the default dict has not been set set it up with the first label set available. | ||
| super().__setattr__("default_dict", l) | ||
| # self.list_of_labels.append(name) | ||
| super().__setattr__(name, l) | ||
|
|
||
| @property | ||
| def list_of_labels(self) -> list[str]: | ||
| return [k for k, v in self.__dict__.items() if isinstance(v, Labels)] | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't like the idea of a default dict but if you have one I think you should have something like: def __getitem__(self, key):
return self.default_dict[key]
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. interesting idea lets discuss that |
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
docstring please
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also why is it
Labelsrather thanStaticStringValueDictfor example?