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

Feature/parameter: Add parameter dialog #246

Merged
merged 8 commits into from
Oct 25, 2023

Conversation

buganini
Copy link
Contributor

@buganini buganini commented Oct 21, 2023

Requirement:
pip3 install QPUIQ==0.3

@buganini buganini changed the title Feature/parameter Feature/parameter: Add parameter dialog Oct 21, 2023
Copy link
Member

@yungyuc yungyuc left a comment

Choose a reason for hiding this comment

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

Thanks, @buganini . Some items to change:

  • Use C++ singleton for globals
  • Use dynamic hierarchical toggles
  • CI needs to be clean

Discussions:

Is it possible to have a simple unit test?

namespace modmesh
{

static int int64V = 5566;
Copy link
Member

Choose a reason for hiding this comment

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

I think the global static data should be managed by a singleton. The singleton will make it possible for testing in Python (pytest).

static std::shared_ptr<pybind11::list> paramsList = nullptr;

template <typename T>
void addParam(const char *key, T *ptr) {
Copy link
Member

Choose a reason for hiding this comment

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

Would it be possible to store the parameter information in the hierarchical dynamic toggle:

def test_get_value(self):
?

More information about modmesh toggles:

* The toggle system for modmesh. There are 3 types of toggles:

@yungyuc yungyuc added the viewer Visualize stuff label Oct 21, 2023
@buganini
Copy link
Contributor Author

image

Copy link
Member

@yungyuc yungyuc left a comment

Choose a reason for hiding this comment

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

  • Please add copyright notice to Python files under the modmesh package

@@ -0,0 +1,63 @@
from PUI.PySide6 import *
Copy link
Member

Choose a reason for hiding this comment

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

Please add license text.

@buganini
Copy link
Contributor Author

buganini commented Oct 22, 2023

clang-format/flake8 issues are fixed

@buganini buganini requested a review from yungyuc October 24, 2023 08:28
@yungyuc yungyuc merged commit 6e605c0 into solvcon:master Oct 25, 2023
11 checks passed
@yungyuc
Copy link
Member

yungyuc commented Oct 25, 2023

I merged the code so that we can move on. A follow up ticket #247 is filed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
viewer Visualize stuff
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants