Skip to content
This repository has been archived by the owner on Oct 24, 2024. It is now read-only.

Configuration integration #184

Closed
wants to merge 16 commits into from

Conversation

ZacZhangzhuo
Copy link
Collaborator

@ZacZhangzhuo ZacZhangzhuo commented Oct 20, 2023

CHANGELOG

  • Remove the scripts folder as it is duplicated with the examples folder, and the other COMPAS packages do not contain this folder.
  • Add example example_custom_keys.py in the examples/control.
  • Update the example files in the tutorial.
  • Update the software architecture diagram in the documentation.
  • Update the UI naming conventions diagram in the documentation.
  • Add the Tutorial Software Concepts section in the documentation.
  • Add the Tutorial Configuration section in the documentation.
  • Action class which controls all the key actions.
  • Categorize all the view settings into one config file.

Checklist

  • I added a line to the CHANGELOG.md file in the Unreleased section under the most fitting heading (e.g. Added, Changed, Removed).
  • I ran all tests on my computer and it's all green (i.e. invoke test).
  • I ran lint on my computer and there are no errors (i.e. invoke lint).
  • I added new functions/classes and made them available on a second-level import, e.g. compas.datastructures.Mesh.
  • I have added tests that prove my fix is effective or that my feature works.
  • I have added the necessary documentation (if appropriate)

Quick Description

  • Key customization possible.
    2023-10-10-20-19-00-52

  • All the settings are configuration-based.
    image

  • Diagrams and new examples for the documentation.
    image

Have a good weekend :)

@ZacZhangzhuo ZacZhangzhuo added documentation Improvements or additions to documentation enhancement New feature or request labels Oct 20, 2023
@ZacZhangzhuo
Copy link
Collaborator Author

I went through invoke lint and invoke test on my machine. But I still got build errors:

Cannot load backend 'Qt5Agg' which requires the 'qt5' interactive framework, as 'headless' is currently running

Looks like an error unrelated to my change.
https://stackoverflow.com/questions/56129786/cannot-load-backend-qt5agg-which-requires-the-qt5-interactive-framework-as

Maybe need help from @Licini

Copy link
Collaborator

@Licini Licini left a comment

Choose a reason for hiding this comment

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

Just some initial feedbacks here and there, could you rebase this branch on the latest main, so that the file change history of example-files don't get mixed in ?
A quick tutorial here:
https://www.youtube.com/watch?v=_UZEXUrj-Ds

PS: a general advice is that this PR is too large, I understand it is difficult for structural changes like this, in the future plz break it down as much as you can, a rule of thumb is to make each PR under 10 files (for non-repetitive edits).

@@ -1,10 +1,12 @@
import os
from .helpers import * # noqa: F401, F403
Copy link
Collaborator

Choose a reason for hiding this comment

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

You don't need this here. And it is the reason is the test failed

@@ -35,4 +37,4 @@ def new_util_find_library(name):
util.find_library = new_util_find_library


__all__ = ["HOME", "DATA", "DOCS", "TEMP", "DATA_OBJECT", "register"]
__all__ = ["HOME", "DATA", "DOCS", "TEMP", "DATA_OBJECT", "register"] # noqa: F405
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why is this added?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Could you elaborate a bit what does helper folder do? (we can discuss in person if too much to write)

@@ -16,6 +14,8 @@

from functools import partial

from typing import Literal, Tuple
Copy link
Collaborator

Choose a reason for hiding this comment

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

move it up and keep it consistent with previous import?

Comment on lines 166 to 197
if title:
config["app"]["title"] = title
if version:
config["app"]["version"] = version
if width:
config["app"]["width"] = width
if height:
config["app"]["height"] = height

if viewmode:
config["view"]["viewmode"] = viewmode
if show_grid:
config["view"]["show_grid"] = show_grid

if enable_sidebar:
config["sidebar"]["enable_sidebar"] = enable_sidebar

if enable_sidedock1:
config["sidedocks"]["enable_sidedock1"] = enable_sidedock1
if enable_sidedock2:
config["sidedocks"]["enable_sidedock2"] = enable_sidedock2
if enable_sceneform:
config["sidedocks"]["enable_sceneform"] = enable_sceneform
if enable_propertyform:
config["sidedocks"]["enable_propertyform"] = enable_propertyform

if show_flow:
config["flow"]["show_flow"] = show_flow
if flow_view_size:
config["app"]["flow_view_size"] = flow_view_size
if flow_auto_update:
config["app"]["flow_auto_update"] = flow_auto_update
Copy link
Collaborator

Choose a reason for hiding this comment

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

It is safer to use if xxx is not None:

from .view_capture import ViewCapture # noqa : F401


def mouse_key_check(event, key_status: Dict, mouse_key: Dict):
Copy link
Collaborator

Choose a reason for hiding this comment

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

These functions should be move to a file, like checks.py

from qtpy import QtCore


def supported_keys():
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why not directly a variable like SUPPORTED_KEYS = {...} instated of function?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Now I think about this, this file should be in scriptes/, and all example files currently there should be moved to a new folder called examples/, but let's make that a separate PR later

@ZacZhangzhuo ZacZhangzhuo mentioned this pull request Oct 23, 2023
2 tasks
@ZacZhangzhuo
Copy link
Collaborator Author

Aha, then this should be good @Licini . My fork is up-to-date now.

@ZacZhangzhuo
Copy link
Collaborator Author

close since it has been separately integrated

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
documentation Improvements or additions to documentation enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Concept Diagram Check Mouse and keyboard combination profile key actions
2 participants