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

[PATH]: Refactor/path module naming cleanup #7440

Merged
merged 33 commits into from Oct 12, 2022

Conversation

mlampert
Copy link
Contributor

@mlampert mlampert commented Sep 4, 2022

Restructure the source code to bring some order into the python modules and make the code more digestible. It does away with most of the naming redundancy and clearly separates business logic from UI dependent code.

This also combines the C++ and python backed object classes in a single namespace. From the python side Log and Geom are pulled into the top level Path namespace due to their universal usage. All other modules are in sub-modules.

Going forward it should be noticeable if a module violates the dependency tree, introduces cyclic references or tries to pull UI logic into the backend business logic. Also, the new structure should be extensible in the future.

@github-actions github-actions bot added the WB CAM Related to the CAM/Path Workbench label Sep 4, 2022
@berndhahnebach
Copy link
Contributor

pipeline status for feature branch PR_7440. Pipeline 630601568 was triggered at 9b58e67. All CI branches and pipelines.

@luzpaz luzpaz added the Refactor PRs that only refactor code label Sep 4, 2022
@berndhahnebach
Copy link
Contributor

pipeline status for feature branch PR_7440. Pipeline 631723654 was triggered at 0a436c9. All CI branches and pipelines.

Copy link
Contributor

@GeneGH GeneGH left a comment

Choose a reason for hiding this comment

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

Quick initial look seems to work correctly, with a couple of exceptions.

Attempts at 3D Surface and 3D Waterline give error messages.

_SurfaceSupport.ERROR: PSF.isReady() no "Op.Surface" method.
Surface.ERROR: Failed to pre-process model and/or selected face(s).

_SurfaceSupport.ERROR: PSF.isReady() no "Op.Waterline" method.
Waterline.ERROR: Unable to pre-process obj.Base.

@mlampert mlampert force-pushed the refactor/path-module-naming-cleanup branch from 0a436c9 to 93112e6 Compare September 10, 2022 06:57
@berndhahnebach
Copy link
Contributor

pipeline status for feature branch PR_7440. Pipeline 636513408 was triggered at 93112e6. All CI branches and pipelines.

@luzpaz
Copy link
Contributor

luzpaz commented Sep 10, 2022

Shall I build a snap for testers of this PR ?

@mlampert mlampert force-pushed the refactor/path-module-naming-cleanup branch from 93112e6 to c83a9a8 Compare September 12, 2022 03:46
@berndhahnebach
Copy link
Contributor

pipeline status for feature branch PR_7440. Pipeline 637146634 was triggered at c83a9a8. All CI branches and pipelines.

@mlampert mlampert force-pushed the refactor/path-module-naming-cleanup branch from c83a9a8 to b759d47 Compare September 16, 2022 02:52
@berndhahnebach
Copy link
Contributor

pipeline status for feature branch PR_7440. Pipeline 641904430 was triggered at b759d47. All CI branches and pipelines.

@mlampert mlampert force-pushed the refactor/path-module-naming-cleanup branch from b759d47 to b3b3403 Compare September 20, 2022 04:05
@berndhahnebach
Copy link
Contributor

pipeline status for feature branch PR_7440. Pipeline 644924193 was triggered at b3b3403. All CI branches and pipelines.

@mlampert mlampert force-pushed the refactor/path-module-naming-cleanup branch from b3b3403 to 304ed0b Compare September 24, 2022 03:07
@berndhahnebach
Copy link
Contributor

pipeline status for feature branch PR_7440. Pipeline 649502278 was triggered at 304ed0b. All CI branches and pipelines.

@mlampert mlampert force-pushed the refactor/path-module-naming-cleanup branch from 304ed0b to 351e3ad Compare September 24, 2022 03:40
@berndhahnebach
Copy link
Contributor

pipeline status for feature branch PR_7440. Pipeline 649511076 was triggered at 351e3ad. All CI branches and pipelines.

@mlampert mlampert force-pushed the refactor/path-module-naming-cleanup branch from 351e3ad to 005b0d3 Compare September 24, 2022 04:54
@berndhahnebach
Copy link
Contributor

pipeline status for feature branch PR_7440. Pipeline 649530520 was triggered at 005b0d3. All CI branches and pipelines.

@berndhahnebach
Copy link
Contributor

pipeline status for feature branch PR_7440. Pipeline 649558764 was triggered at cc19425. All CI branches and pipelines.

@mlampert mlampert force-pushed the refactor/path-module-naming-cleanup branch from cc19425 to 780699d Compare September 25, 2022 08:13
@berndhahnebach
Copy link
Contributor

pipeline status for feature branch PR_7440. Pipeline 649996458 was triggered at 780699d. All CI branches and pipelines.

@sliptonic
Copy link
Member

@Floriansimmer Is it possible to get any more information about why the github CI is failing? These unit tests pass locally on multiple platforms.

@mlampert mlampert force-pushed the refactor/path-module-naming-cleanup branch from 780699d to d07cb93 Compare September 30, 2022 01:08
@berndhahnebach
Copy link
Contributor

pipeline status for feature branch PR_7440. Pipeline 654669506 was triggered at d07cb93. All CI branches and pipelines.

@mlampert mlampert force-pushed the refactor/path-module-naming-cleanup branch from d07cb93 to f3f2c76 Compare October 1, 2022 18:47
@berndhahnebach
Copy link
Contributor

pipeline status for feature branch PR_7440. Pipeline 656041787 was triggered at f3f2c76. All CI branches and pipelines.

@berndhahnebach
Copy link
Contributor

pipeline status for feature branch PR_7440. Pipeline 656475209 was triggered at 085c81b. All CI branches and pipelines.

@mlampert mlampert force-pushed the refactor/path-module-naming-cleanup branch from 085c81b to 1742dfc Compare October 3, 2022 03:31
@mlampert mlampert force-pushed the refactor/path-module-naming-cleanup branch from dc44fce to c74a0ad Compare October 12, 2022 04:43
@berndhahnebach
Copy link
Contributor

pipeline status for feature branch PR_7440. Pipeline 664431748 was triggered at c74a0ad. All CI branches and pipelines.

@sliptonic sliptonic merged commit 62a4503 into FreeCAD:master Oct 12, 2022
@mlampert mlampert deleted the refactor/path-module-naming-cleanup branch October 13, 2022 02:30
@wwmayer
Copy link
Contributor

wwmayer commented Oct 13, 2022

Since this PR is merged I get this error message when starting FreeCAD:

Path WB not found

In the combo box the Path workbench is still listed and when I try to activate this error is raised:

module 'Path.Tool' has no attribute 'Controller'
Traceback (most recent call last):
  File "<string>", line 82, in Initialize
  File "/home/user/Projects/build_clang/Mod/Path/Path/Main/Gui/JobCmd.py", line 29, in <module>
    import Path.Main.Gui.JobDlg as PathJobDlg
  File "/home/user/Projects/build_clang/Mod/Path/Path/Main/Gui/JobDlg.py", line 29, in <module>
    import Path.Main.Job as PathJob
  File "/home/user/Projects/build_clang/Mod/Path/Path/Main/Job.py", line 31, in <module>
    import Path.Tool.Controller as PathToolController
  File "/home/user/Projects/build_clang/Mod/Path/Path/Tool/Controller.py", line 351, in <module>
    from Path.Tool.Gui.Controller import ViewProvider
  File "/home/user/Projects/build_clang/Mod/Path/Path/Tool/Gui/Controller.py", line 30, in <module>
    import Path.Tool.Controller as PathToolController

@mlampert
Copy link
Contributor Author

mlampert commented Oct 14, 2022 via email

@sliptonic sliptonic mentioned this pull request Oct 16, 2022
7 tasks
@wwmayer
Copy link
Contributor

wwmayer commented Oct 21, 2022

I was able to solve the issue. Previously I've only deleted Mod/Path of my build directory but after realizing that another build works fine I now have deleted src/Mod/Path as well and after rebuilding it the issue is gone.

@luzpaz
Copy link
Contributor

luzpaz commented Nov 8, 2022

Did this PR create this regression? https://forum.freecadweb.org/viewtopic.php?p=639162#p639162

@dresco
Copy link
Contributor

dresco commented Nov 8, 2022

Did this PR create this regression? https://forum.freecadweb.org/viewtopic.php?p=639162#p639162

Fwiw, appears to have happened in this commit

@mlampert
Copy link
Contributor Author

mlampert commented Nov 8, 2022

It seems I messed up a rebase - will fix it tonight - sorry about that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Refactor PRs that only refactor code WB CAM Related to the CAM/Path Workbench
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

8 participants