Repository navigation
feat[canvas]: Patch the orange-canvas to add an export and import menus as ewoks json workflow #503
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
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,102 @@ | ||
| import io | ||
| import logging | ||
| import os | ||
| import tempfile | ||
| from pathlib import Path | ||
|
|
||
| from AnyQt.QtWidgets import QAction | ||
| from AnyQt.QtWidgets import QFileDialog | ||
| from AnyQt.QtWidgets import QMenu | ||
| from orangecanvas.application.canvasmain import CanvasMainWindow | ||
|
|
||
| from ..workflows.owscheme import ewoks_to_ows | ||
| from ..workflows.owscheme import ows_to_ewoks | ||
|
|
||
| _logger = logging.getLogger(__name__) | ||
| _FILTER = "Ewoks workflow (*.json *.yml *.yaml)" | ||
| _patched = False | ||
|
|
||
|
|
||
| def _import_ewoks_workflow(window: CanvasMainWindow) -> None: | ||
|
|
||
| from ...bindings import load_graph # Lazy import to avoid circular import | ||
|
|
||
| filename, _ = QFileDialog.getOpenFileName( | ||
| window, "Import Ewoks workflow", "", _FILTER | ||
| ) | ||
| if not filename: | ||
| return | ||
| try: | ||
| graph = load_graph(filename) | ||
| with tempfile.TemporaryDirectory() as tmpdir: | ||
| ows = os.path.join(tmpdir, "imported.ows") | ||
| ewoks_to_ows(graph, ows, error_on_duplicates=False) | ||
| # Opens in this window if it is empty, else in a new one, | ||
| # without associating the temporary path with the document. | ||
| window.open_example_scheme(ows) | ||
| except Exception as e: | ||
| _logger.exception("Ewoks import failed") | ||
| window.output_view().write(f"Ewoks import failed: {e}\n") | ||
|
|
||
|
|
||
| def _export_ewoks_workflow(window: CanvasMainWindow) -> None: | ||
|
|
||
| from ...bindings import save_graph # Lazy import to avoid circular import | ||
|
|
||
| filename, _ = QFileDialog.getSaveFileName( | ||
| window, "Export Ewoks workflow", "", _FILTER | ||
| ) | ||
| if not filename: | ||
| return | ||
| if not Path(filename).suffix: | ||
| filename += ".json" | ||
| try: | ||
| scheme = window.current_document().scheme() | ||
| buffer = io.BytesIO() | ||
| scheme.save_to(buffer, pretty=True, pickle_fallback=True) | ||
| buffer.seek(0) | ||
| graph = ows_to_ewoks(buffer) | ||
| save_graph(graph, filename) | ||
| except Exception as e: | ||
| _logger.exception("Ewoks export failed") | ||
| window.output_view().write(f"Ewoks export failed: {e}\n") | ||
|
|
||
|
|
||
| def _add_actions(window: CanvasMainWindow) -> None: | ||
| menu = window.menuBar().findChild(QMenu, "file-menu") | ||
| if menu is None: | ||
| return | ||
| import_action = QAction( | ||
| "Import Ewoks workflow...", window, objectName="action-import-ewoks" | ||
| ) | ||
| import_action.triggered.connect(lambda: _import_ewoks_workflow(window)) | ||
| export_action = QAction( | ||
| "Export Ewoks workflow...", window, objectName="action-export-ewoks" | ||
| ) | ||
| export_action.triggered.connect(lambda: _export_ewoks_workflow(window)) | ||
|
|
||
| before = menu.findChild(QAction, "open-actions-separator") | ||
| for action in (import_action, export_action): | ||
| if before is not None: | ||
| menu.insertAction(before, action) | ||
| else: | ||
| menu.addAction(action) | ||
|
|
||
|
|
||
| def add_ewoks_import_export_actions() -> None: | ||
| """ | ||
| Add Ewoks import/export to the File menu of every canvas window. | ||
| """ | ||
| global _patched | ||
| if _patched: | ||
| return | ||
| _patched = True | ||
|
|
||
| # CanvasMainWindow is instantiated both in Orange3 or in Oasys2 and there is no public interface given. | ||
| original_init = CanvasMainWindow.__init__ | ||
|
|
||
| def __init__(self, *args, **kwargs): | ||
| original_init(self, *args, **kwargs) | ||
| _add_actions(self) | ||
|
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. Adding actions is a good idea. Perhaps there are other ways to do this without patching? Not sure.
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. Yes. One question is: how much different is it from the already existing
Contributor
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. yes we save as a json ewoks workflow equivalent to an ewoks convert .ows <-> .json
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. Sorry, I wasn’t clear. My question is whether we should add new actions or, if possible, reuse the existing one and adapt the current code (QDialog, save/load functions).
Contributor
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. I can investigate first if this is possible (Without going to deep into monkey patching / changing orange canvas api) But in my opinion. It looks normal for me to have an "export import" pattern. --> The native way to save load orange workflow is ows. For me this is the same as word. You save / load as docx but you can export import as open office format
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 agree with you on the technical side. That said, I keep wondering: what is the user actually expecting here? Could it be confusing for them to have to choose between saving and exporting? And are we at risk of ending up with too many actions? I’m not pushing for one option over the other, but I think we should at least ask ourselves these questions first. The technical side can come second.
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.
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. See #504 As expected, four things need patching: Also, the opening for OASYS needs to be patched separately since the implementation is slightly different. What do you think @payno ?
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. Hard to say. Modifying But def open_scheme():
dlg = self._open_workflow_dialog()
dlg.setParent(dialog, Qt.Dialog)
dlg.fileSelected.connect(self.open_scheme_file)
dlg.accepted.connect(dialog.accept)
dlg.exec()
...
def open_scheme(self):
# type: () -> None
"""
Open a user selected workflow in a new window.
"""
dlg = self._open_workflow_dialog()
dlg.fileSelected.connect(self.open_scheme_file)
dlg.exec()
def open_and_freeze_scheme(self):
# type: () -> None
"""
Open a user selected workflow file in a new window and freeze
signal propagation.
"""
dlg = self._open_workflow_dialog()
dlg.fileSelected.connect(partial(self.open_scheme_file, freeze=True))
dlg.exec()So I agree this is more code to maintain. But at a first glance it could be 'clean' enough.
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. Let's discuss this in the next workflow meeting to reach a collective agreement then. In the mean time, I think we can merge this PR |
||
|
|
||
| CanvasMainWindow.__init__ = __init__ | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,61 @@ | ||
| import json | ||
|
|
||
| from ewoksorange.gui.canvas import menu | ||
| from ewoksorange.gui.canvas.handler import OrangeCanvasHandler | ||
|
|
||
|
|
||
| def test_exported_json_matches_manual_workflow( | ||
| tmp_path, monkeypatch, ewoksorange_qtapp | ||
| ): | ||
| destination_path = tmp_path / "exported_workflow.json" | ||
| graph = { | ||
| "nodes": [ | ||
| { | ||
| "id": "source", | ||
| "task_type": "class", | ||
| "task_identifier": "ewokscore.tests.examples.tasks.sumtask.SumTask", | ||
| }, | ||
| { | ||
| "id": "sink", | ||
| "task_type": "class", | ||
| "task_identifier": "ewokscore.tests.examples.tasks.sumtask.SumTask", | ||
| }, | ||
| ], | ||
| "links": [ | ||
| { | ||
| "source": "source", | ||
| "target": "sink", | ||
| "data_mapping": [{"source_output": "result", "target_input": "a"}], | ||
| } | ||
| ], | ||
| } | ||
|
|
||
| source_path = tmp_path / "source_workflow.json" | ||
|
|
||
| with open(source_path, "w") as f: | ||
| json.dump(graph, f) | ||
|
|
||
| # Here i import to the canvas then re export it and verify the exported json. | ||
|
|
||
| with OrangeCanvasHandler() as handler: | ||
| monkeypatch.setattr( | ||
|
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. Not a fan of
Contributor
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. yep me too but that's the better way to bypass the dialog |
||
| menu.QFileDialog, | ||
| "getOpenFileName", | ||
| lambda *args, **kwargs: (str(source_path), ""), | ||
| ) | ||
|
|
||
| menu._import_ewoks_workflow(handler.canvas) | ||
|
|
||
| monkeypatch.setattr( | ||
| menu.QFileDialog, | ||
| "getSaveFileName", | ||
| lambda *_args, **_kwargs: (str(destination_path), ""), | ||
| ) | ||
| menu._export_ewoks_workflow(handler.canvas) | ||
|
|
||
| with open(destination_path) as f: | ||
| exported_workflow = json.load(f) | ||
|
|
||
| assert exported_workflow["nodes"][0]["id"] == "source" | ||
| assert exported_workflow["nodes"][1]["id"] == "sink" | ||
| assert exported_workflow["links"] == graph["links"] | ||

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.
I have verified in the code and Oasys / Orange looks to both use
orangecanvas.application.canvasmaiCanvasMainWindowso it should worksThere 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.
Put this as comment perhaps?