Support tag for builder app #323
Conversation
See also: #322
iquip/apps/explorer.py
Outdated
| Returns: | ||
| See BuilderInfoDialog. |
There was a problem hiding this comment.
Let's explicitly document that the returned tuple is (cls, tag).
You can still say "See BuilderInfoDialog."
iquip/apps/explorer.py
Outdated
| tag: Additive tag for multiple builder apps. | ||
| """ | ||
|
|
||
| def __init__(self, experimentInfos: List[str], parent: Optional[QWidget] = None): |
There was a problem hiding this comment.
Why is the name experimentInfos instead of clsNames for example?
There was a problem hiding this comment.
Based on your usage, I think it is not List but Iterable.
There was a problem hiding this comment.
Why is the name experimentInfos instead of clsNames for example?
Umm.. I also think clsNames is much proper. I will take it.
iquip/apps/explorer.py
Outdated
| self.qiwiscall.createApp( | ||
| name=f"builder - {self.selectedExperimentPath}:{experimentClsName}", | ||
| name=f"builder {tag} - {self.selectedExperimentPath}:{cls}", |
There was a problem hiding this comment.
This will make 2 spaces for an empty tag.
See also: #322
| Args: | ||
| See thread.ExperimentInfoThread.fetched signal. |
There was a problem hiding this comment.
Now this is actually not a proper redirection, but I think it is pretty intuitive enough. If you agree with it, LGTM. You can update the docstring as well.
This closes #322.
Now, one can set tags for experiment classes, thus multiple builder apps for the same experiment class are available.