Skip to content
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

Support tag for builder app #323

Open
wants to merge 4 commits into
base: develop
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
86 changes: 61 additions & 25 deletions iquip/apps/explorer.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,8 @@
import requests
from PyQt5.QtCore import QObject, Qt, QThread, pyqtSlot, pyqtSignal
from PyQt5.QtWidgets import (
QInputDialog, QPushButton, QTreeWidget, QTreeWidgetItem, QVBoxLayout, QWidget
QComboBox, QDialog, QHBoxLayout, QLineEdit, QPushButton, QTreeWidget, QTreeWidgetItem,
QVBoxLayout, QWidget,
)

import qiwis
Expand Down Expand Up @@ -96,6 +97,48 @@ def run(self):
self.fetched.emit(experimentList, self.widget)


class BuilderInfoDialog(QDialog):
"""Dialog for setting builder info.

There are two types of info.
cls: Target experiment class name.
tag: Additive tag for multiple builder apps.
"""

def __init__(self, experimentInfos: List[str], parent: Optional[QWidget] = None):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is the name experimentInfos instead of clsNames for example?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Based on your usage, I think it is not List but Iterable.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is the name experimentInfos instead of clsNames for example?

Umm.. I also think clsNames is much proper. I will take it.

"""Extended.

Args:
See thread.ExperimentInfoThread.fetched signal.
Comment on lines +111 to +112
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

"""
super().__init__(parent=parent)
# widgets
self.comboBox = QComboBox(self)
self.comboBox.addItems(experimentInfos)
self.lineEdit = QLineEdit(self)
self.lineEdit.setPlaceholderText("Tag")
self.okButton = QPushButton("OK", self)
self.okButton.clicked.connect(self.accept)
self.cancelButton = QPushButton("Cancel", self)
self.cancelButton.clicked.connect(self.reject)
# layout
buttonLayout = QHBoxLayout()
buttonLayout.addWidget(self.okButton)
buttonLayout.addWidget(self.cancelButton)
layout = QVBoxLayout(self)
layout.addWidget(self.comboBox)
layout.addWidget(self.lineEdit)
layout.addLayout(buttonLayout)

def getBuilderInfo(self) -> Tuple[str, str]:
"""Returns the configured builder info.

Returns:
See BuilderInfoDialog.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's explicitly document that the returned tuple is (cls, tag).
You can still say "See BuilderInfoDialog."

"""
return self.comboBox.currentText(), self.lineEdit.text()


class ExplorerApp(qiwis.BaseApp):
"""App for showing the experiment list and opening an experiment.

Expand Down Expand Up @@ -216,54 +259,47 @@ def fetchExperimentInfo(self, item: QTreeWidgetItem):
self.proxy_port,
self
)
self.experimentInfoThread.fetched.connect(self.selectExperimentCls,
self.experimentInfoThread.fetched.connect(self.openBuilderInfoDialog,
type=Qt.QueuedConnection)
self.experimentInfoThread.finished.connect(self.experimentInfoThread.deleteLater)
self.experimentInfoThread.start()

@pyqtSlot(dict)
def selectExperimentCls(self, experimentInfos: Dict[str, ExperimentInfo]):
"""Selects an experiment class to be opened as a builder.
def openBuilderInfoDialog(self, experimentInfos: Dict[str, ExperimentInfo]):
"""Opens a dialog for setting builder info.

After selected, self.openBuilder() is called to open a builder.

If there is only one class, it is selected automatically without showing a QInputDialog.
If no class is selected, nothing happens.
If the dialog is confirmed, self.openBuilder() is called with the configured builder info.
Otherwise, nothing happens.

Args:
See thread.ExperimentInfoThread.fetched signal.
"""
if len(experimentInfos) > 1:
cls, ok = QInputDialog().getItem(None, "Select an experiment class",
"Experiment class: ", experimentInfos, editable=False)
if not ok:
return
else:
cls = next(iter(experimentInfos))
self.openBuilder(cls, experimentInfos[cls])

def openBuilder(
self,
experimentClsName: str,
experimentInfo: ExperimentInfo
):
dialog = BuilderInfoDialog(experimentInfos)
if dialog.exec_():
cls, tag = dialog.getBuilderInfo()
self.openBuilder(cls, tag, experimentInfos[cls])


def openBuilder(self, cls: str, tag: str, experimentInfo: ExperimentInfo):
"""Opens the experiment builder with its information.

The experiment is guaranteed to be the correct experiment file.

Args:
experimentClsName: The class name of the experiment.
cls, tag: See BuilderInfoDialog.
experimentInfo: The experiment information. See protocols.ExperimentInfo.
"""
if tag:
tag = f"({tag})"
self.qiwiscall.createApp(
name=f"builder - {self.selectedExperimentPath}:{experimentClsName}",
name=f"builder {tag} - {self.selectedExperimentPath}:{cls}",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will make 2 spaces for an empty tag.

info=qiwis.AppInfo(
module="iquip.apps.builder",
cls="BuilderApp",
pos="center",
args={
"experimentPath": self.selectedExperimentPath,
"experimentClsName": experimentClsName,
"experimentClsName": cls,
"experimentInfo": experimentInfo
}
)
Expand Down
6 changes: 3 additions & 3 deletions tests/test_explorer.py
Original file line number Diff line number Diff line change
Expand Up @@ -200,16 +200,16 @@ def test_open_builder(self):
app.selectedExperimentPath = "experimentPath"
experimentInfo = protocols.ExperimentInfo("name", {"arg0": "value0"})
with mock.patch.object(app, "qiwiscall") as mocked_qiwiscall:
app.openBuilder("experimentClsName", experimentInfo)
app.openBuilder("cls", "tag", experimentInfo)
mocked_qiwiscall.createApp.assert_called_with(
name="builder - experimentPath:experimentClsName",
name="builder (tag) - experimentPath:cls",
info=qiwis.AppInfo(
module="iquip.apps.builder",
cls="BuilderApp",
pos="center",
args={
"experimentPath": "experimentPath",
"experimentClsName": "experimentClsName",
"experimentClsName": "cls",
"experimentInfo": experimentInfo
}
)
Expand Down