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

Refactor to combine RApplication and RMainWindow #174

Merged

Conversation

yungyuc
Copy link
Member

@yungyuc yungyuc commented Dec 10, 2022

By combining RApplication and RMainWindow into RManager, QApplication and QMainWindow can be exposed without being inherited and accessed using PySide6 in Python. It will further help development.

modmesh.view.app is renamed to modmesh.view.mgr. It avoids the confusing API between the manager object and the modmesh.app plugin-ish sub-system.

The 1D Euler app is enhanced to allow to be created without starting the animation automatically. Stepping in the plotting is also possible.

Objective: enhance the euler1d app with more control

* The original `Plotter` is renamed to `Controller`.
* The code directly operate matplotlib is factored to a new `Plot` class.
* A new data class `QuantityLine` is created to hold a pair of an analytical line and a numerical line for a quantity.
* Keep RApplication and RMainWindow inherit from QObject rather than the two derived classes.  The composite structure makes QApplication and QMainWindow objects accessislble from Python usin PySide6.
* RApplication and RMainWindow should be combined to be a new class RManager.  It should be done in the next checkin.
* This is a precursor of turning RMainWindow and RApplication into a single RManager class.
* Python modmesh.view.app is also renamed to modmesh.view.mgr.
@yungyuc yungyuc added refactor Change code without changing tests viewer Visualize stuff labels Dec 10, 2022
@yungyuc yungyuc self-assigned this Dec 10, 2022
Copy link
Member Author

@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.

@tychuang1211 Would you have time to take a look tonight or tomorrow? If not I will directly merge.

@@ -26,12 +26,11 @@
* POSSIBILITY OF SUCH DAMAGE.
*/

#include <modmesh/view/RMainWindow.hpp> // Must be the first include.
#include <modmesh/view/RManager.hpp> // Must be the first include.
Copy link
Member Author

Choose a reason for hiding this comment

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

Code in RApplication and RMainWindow is merged here. The diff is messy. It's probably too hard for review.

@@ -125,6 +125,9 @@ struct qt_type_caster
}

QT_TYPE_CASTER(QWidget, _("QWidget"));
QT_TYPE_CASTER(QCoreApplication, _("QCoreApplication"));
Copy link
Member Author

Choose a reason for hiding this comment

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

Add casters for QApplication and QMainWindow.

//
;
}

wrapper_type & wrap_basic_qt()
wrapper_type & wrap_widget()
Copy link
Member Author

Choose a reason for hiding this comment

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

Organize wrapping functions in member functions.

Copy link
Member Author

Choose a reason for hiding this comment

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

This is for widgets

})
//
;

return *this;
}

}; /* end class WrapRMainWindow */
wrapper_type & wrap_app()
Copy link
Member Author

Choose a reason for hiding this comment

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

This is for the "app" sub-system.


WrapRApplication(pybind11::module & mod, char const * pyname, char const * pydoc)
: root_base_type(mod, pyname, pydoc)
wrapper_type & wrap_mainWindow()
Copy link
Member Author

Choose a reason for hiding this comment

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

This is to make it work like QMainWindow

self.num.figure.canvas.draw()


class Plot:
Copy link
Member Author

Choose a reason for hiding this comment

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

Plot holds the major logic for plotting.

ndata=shocktube.svr.velocity[::2])


class Controller:
Copy link
Member Author

Choose a reason for hiding this comment

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

Controller holds the major logic for the calculation.

view.mgr.pycon.writeToHistory('\n')


def run(interval=10, max_steps=50, **kw):
Copy link
Member Author

Choose a reason for hiding this comment

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

run() function is kept to work as a simple driver.

@@ -38,13 +38,13 @@
class ViewTC(unittest.TestCase):

def test_import(self):
self.assertTrue(hasattr(modmesh.view, "app"))
self.assertTrue(hasattr(modmesh.view, "mgr"))
Copy link
Member Author

Choose a reason for hiding this comment

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

Rename app to mgr. This is less confusing a name!

@yungyuc yungyuc merged commit 3542288 into solvcon:master Dec 12, 2022
@yungyuc yungyuc deleted the refactor/combine-application-mainwindow branch December 12, 2022 09:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
refactor Change code without changing tests viewer Visualize stuff
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant