Eomys / pyleecan

Electrical engineering open-source software providing a user-friendly, unified, flexible simulation framework for the multiphysic design and optimization of electrical machines and drives
https://www.pyleecan.org
Apache License 2.0
156 stars 131 forks source link

GUI for class generator #659

Closed EmileDvs closed 10 months ago

EmileDvs commented 1 year ago

Hello everyone,

A long time ago when I was contributing a bit more to pyleecan (:-)) I had the idea to provide a GUI to help handling the majestic great grand pyleecan class generator, especially for people who start playing with pyleecan and try to create their first objects.

I have recently coded a basic GUI with PySide2, which I prefer to say remains quite modest regarding all the capatibilities of the mighty class generator, but yet enables to:

Here is a screenshot of the interface: image

I'm posting the pull request so everybody can look at it and decide whether it can be useful or not and thus should be merged or not. Of course this pull request is not ready to be merged, as there are still some "minor details" to fix:

Besides I intend to use the interface in a second project and that requires extensive object creation and edition so I'll probably encounter some bugs and fix them before this PR can be merged,

Best regards, Emile

BonneelP commented 1 year ago

Hello,

The great grand pyleecan class generator is pleased by your contribution :D This is a great addition to ease the development within pyleecan, thanks !

If I can suggest two additional features, it would be:

I didn't find if there is a "template for new methods" but it would be great that the created file start by "def meth_name(self):" followed by a empty template for docstring and a raise NotImplementedYet (it is maybe already the case).

We are currently focused on preparing the next release of our other project, but we should have time to review this PR and #624 at the end of the month (then we will create a new release of pyleecan).

Best regards, Pierre

EmileDvs commented 1 year ago

Thank you very much Pierre for your answer, I have implemented the features you suggested.

I am now testing the GUI and the first issues that I encounter and am currently solving is the impact of renaming/deleting classes that are parent of others, which make the class generator logically crashing.

This is not technically a big deal but it requires some further development. I have included the children classes in the Metadata table besides the class description and added a message box in case of renaming/deleting class with known children. Now I am tracking the list of children for each class so they can be automatically updated.

Emile

BonneelP commented 1 year ago

Hello,

There is no need to include the children classes in the table (it used to be the case), there is a function in the class generator that already scan all csv to find all daughters of a class ;)

Best regards, Pierre

EmileDvs commented 1 year ago

Hello Pierre,

My last comment was ambiguous: the daughters are only displayed in the interface, they are not editable and not saved in csv files. Of course I relied on the read_all update_all_daughters to fetch children list for each class.

image

Then, when adding a parent to the current class, the current class is automatically added to the list of children of the parent class.

Emile

EmileDvs commented 1 year ago

Hello everyone,

I've used the GUI on another project to create classes and methods and I think it works pretty well right now. I've corrected several bug and issues and I don't think I'll need and add other features on my side, so you can start the review anytime you want.

Best regards, Emile

BonneelP commented 10 months ago

Hello,

Sadly it wasn't anytime we want but anytime we can ^^' I'm very sorry for the delay of the review ! I finally managed to review your PR, thank you very much for your contribution, it will really help speed-up the development time with pyleecan !

I will open a new PR based on yours with few minor improvements (not all are linked to your work, I took the opportunity):

I have one last question: In pyleecan\Generator\run_class_generator_GUI.py, there are path that are linked to your computer, I don't know if we can automatically detect the proper tools but it may be a security issue for you to share local path. Besides, what do you think about adding a "isfile" check in DClassGenerator init ?

When I generate the Ui file of DClassGenerator, I have the following messages: image

Best regards and Happy new year, Pierre (PS, we can carry on the discussion in this PR)

EmileDvs commented 10 months ago

Hi Pierre,

Happy new year to you too :)

It's been a while indeed, I had to refresh my memories about the PR :)

I've noticed that the read_all fonction returns all the parents classes (i.e. parents, grand parents etc.) in the class_dict, which is too much information as the class generator GUI only needs to display the parent, not grand-parents etc. So I've introduced the is_mother_of_mother parameter, set it to True by default to keep the previous behaviour, and set it to false in DClassGenerator to only have the parent class.

Concerning the paths you're right I forgot to erase them from the files, and good idea to check the paths in the init. Do you do it in your PR or do I do it on my branch ?

Finally I don't know how to fix the layoutWidget duplicated names in Qt creator. They are added in the .ui but they are not displayed in Qt creator. Anyway I think it does not really matter since these layouts are not used within the code.

Best regards, Emile

BonneelP commented 10 months ago

Hi Emile,

Ok to keep your modifications for is_update_mother_of_mother then :)

I will take a look to the .ui before merging to check if I can find the layout issue. If not, I will merge anyway I think.

Regarding the path, you can do the modifications on your PR, I will then pull them on my own and merge both PR.

Best regards, Pierre

EmileDvs commented 10 months ago

Hi Pierre,

I did the modifications on my branch, let me know if you need anything else before merging.

Best regards, Emile

BonneelP commented 10 months ago

Great :) I'm merging right now, Thank you for your contribution !

Best regards, Pierre