Refactor + design patterns

[tuxxi]

Ideas for things we should consider refactoring:

• main.py: This is a given, we need to extract MainWindow and isolate code that doesn't include UI into a different class, maybe application or something.

• Folder Structure: This is related to the discussion in #56 about where to store default values. Often python projects or packages will have a package for the "overall" project, which can contain code or data common to all sub-modules. If we choose this refactor, our codebase would look like this:

.
+-- _openMotor
|    +-- _ui
|    +-- _motor
+-- main.py
+-- ... etc

This will solve the current issues with circular imports. Another benefit to this is that we can add new modules, for example the thermo module for Cantera chemical equilibrium which I am planning to tackle soon (maybe post 0.4.0?). This also allows us to post openMotor as a package on pyPI so people can use our libraries and APIs programmatically.

• Style: We should follow PEP 8 style guide wherever possible. We don't have to follow all of it, for example snake_case everywhere is not necessary IMHO, but we should at least consider:

  • class names should be UpperCamelCase
  • line length <= 120 chars (?)
  • two newlines between each class definition
  • .. and more! We can use an auto-formatter for this if you feel inclined, Black is pretty good.

• Docstrings: We should try to include docstrings wherever possible, especially in the fmm code and motorlib. UI docstrings are not as crucial but they help.

• Everything listed in #53, but especially C* and per-motor settings.

More suggestions please! We don't need to tackle everything at once, this is just ideas for now.

[reilleya]

I'm in favor of any standardization we can do. I'm not super familiar with python-specific best practices for package layout, so I'll defer to your opinions on that issue. Do you have any links to reading I can do on typical project layout?

main.py: I have intended to make a separate file for the main window since I added headless mode but hadn't gotten around to it yet. I think I have been pretty good about keeping non-UI code out of the file, but I sure we will uncover some stuff while refactoring.

Folder structure: Looks good to me. Related to this, I was wondering if it made sense to you to add another level of organization to the UI code. I found myself grouping together related classes into single files, like propellant.py containing propellantManager, propellantMenu, and propellantEditor. Should these each have their own file in a folder (submodule) called propellant, or does it make sense to group them into a single file? In either case, I want to establish a naming scheme to ensure that all of these UI components have logical names.

Style: In case you couldn't tell, I'm not a huge fan of snake case. I think camel case reads a lot better, and there is enough camel case in python libraries (especially Qt) I have used that I don't feel bad about sticking with it. I agree that we should use a linter and will look into Black. I'd prefer lines in the 100-120 character range. I feel that 80 character limits can lead to very short variable names that are hard to read.

Docstrings: Agreed, and I figure I should take this one. I will make an issue in 0.3.0 for documenting all of motorlib.

Simulation changes: These changes are the ones I would want to see first, because I was hoping to have configurable limits in v0.2.0 and it doesn't make sense to add them without having them exist on a per-motor basis.

I see some big commits in the near future, and I think I want to get v0.2.0 out before we dive this deep. This is in large part because of the mac redraw issues that it will fix. I guess I could just release a v0.1.1 that resolves those issues, but if we stick to the original scope of v0.2.0 we can get it done soon enough that it won't matter. With this in mind, I think we should add the per-motor configuration stuff described in #53 in for v0.2.0 and push the rest out for v0.3.0. Once v0.2.0 is out, we can do the refactor before we implement any of the new v0.3.0 features. If you want to handle the folder restructuring/imports, I will work on setting up a code formatter once you are done.

[tuxxi]

Folder structure:

Hitchhikers Guide to Python is an excellent resource for general best practices. FWIW I've seen their recommendations in other FOSS projects.

Simulation Changes:

Because erosive-burning #31 has many breaking changes to the simulation modules, IMHO we should merge it as an experiential feature first and then tackle the other changes to the simulation for 0.2.0.

[reilleya]

Thanks, I'll look it over! Sure, as long as simulation can still be run normally I think that is a good solution. We can also gather more testing data from TRF that way, so people can compare the results themselves.

[reilleya]

I have mostly gotten through a refactor of the UI and wanted to see if you had any ideas. My current progress is in the ui_refactor branch. I moved all of the widgets to a new submodule (widgets), and removed all of the wildcard imports in uilib. The problem with circular imports is gone! I also moved preferences handling to a new manager, so that code no longer lives in the MainWindow. Then I used pylint to fix almost all of the style issues in the code. It seems pylint is a bit less strict than black, but it still got things looking a lot cleaner. If that is what we decide to stick with, I will add the .pylintrc file to the repo and maybe make a setup.py command to run it on the entire codebase. The next step I see is moving all of the managers to an application class that also instantiates a MainWindow if it hasn't been run in headless mode. Is that an arrangement that makes sense to you? I think the scope of this issue ends around there unless you see any other important UI refactors, and then I will attack (and document) motorlib for #60. I will document the UI for v0.4.0.

@tuxxi

[reilleya]

This issue can be reopened in the future, but I'm pretty happy with where the code is for now.