[Discussion] Splitting Draft tools into their own modules

A forum dedicated to the Draft, Arch and BIM workbenches development.
Forum rules
Be nice to others! Respect the FreeCAD code of conduct!
Post Reply
vocx
Veteran
Posts: 5197
Joined: Thu Oct 18, 2018 9:18 pm

Re: [Discussion] Splitting Draft tools into their own modules

Post by vocx »

Pull request #2829 uncovered a problem with the way new files are installed in directories. This resulted in the Travis tests failing because certain tools in Draft weren't being found.

Pull request #2844 adds specific INSTALL instructions in the CMakeLists.txt file so that files are installed in the right directories. I didn't do this before because on my system the files were installed correctly just by listing them on the sources. However, it seems that on Travis the files weren't moved to the correct directory. The files weren't placed correctly in the freecad-daily package either. This has been occurring since I moved the Draft tests, in pull request #2727, but back then it didn't cause any problem because it was only test files, not used by other workbenches.

So, for example, auxiliary.py should be in

Code: Select all

Mod/Draft/drafttests/auxiliary.py
but it was installed in

Code: Select all

Mod/Draft/auxiliary.py
Pull request #2844 corrects this problem. It is already merged.

Originally the sequence of merging was this
* Pull request #2823. GuiCommandBase.
* Pull request #2824. Draft_PolarArray.
* Pull request #2825. Draft_CircularArray.
* Pull requests #2829, #2830, #2831, #2832. General utilities, GUI utilities, "todo" class, and "translate" function.
* Pull requests #2840. CMakeLists tweaks.

The first #2823 was already merged, then #2844 was merged to correct the CMakeLists problem. Then, I rebased the rest of the pull requests, solved the merge conflicts, and forced pushed them. So the remaining branches can be merged in this order.
* Pull request #2824. Draft_PolarArray.
* Pull request #2825. Draft_CircularArray.
* Pull requests #2829, #2830, #2831, #2832. General utilities, GUI utilities, "todo" class, and "translate" function.

The last one, #2840, is no longer needed. I added it before I noticed the problem at the beginning of this post. It is completely unnecessary now that #2844 is merged.

----

By the way, one of the Travis tests will still fail. This is due to an unrelated error, which is due to Python 2 not recognizing the utf8 encoding of certain files. For more information see #2848. Basically, all files that have vowel-umlauts (äüö) need to have the proper encoding instruction.
Always add the important information to your posts if you need help. Also see Tutorials and Video tutorials.
To support the documentation effort, and code development, your donation is appreciated: liberapay.com/FreeCAD.
User avatar
yorik
Founder
Posts: 13640
Joined: Tue Feb 17, 2009 9:16 pm
Location: Brussels
Contact:

Re: [Discussion] Splitting Draft tools into their own modules

Post by yorik »

I have started merging these, but it will require some further work from you @vocx as each seem to introduce conflicts to the next ones.
I believe this might likely give some problems afterwards, but we'll be happy that it has been done, so it's best to merge and fix problem as they arise...

I am a bit more back into action now (big life changes ongoing..) so hopefully can help much better.
vocx
Veteran
Posts: 5197
Joined: Thu Oct 18, 2018 9:18 pm

Re: [Discussion] Splitting Draft tools into their own modules

Post by vocx »

yorik wrote: Wed Jan 08, 2020 3:29 pm I have started merging these, but it will require some further work from you @vocx as each seem to introduce conflicts to the next ones.
I believe this might likely give some problems afterwards, but we'll be happy that it has been done, so it's best to merge and fix problem as they arise...
Yes, I just realized the way I structured the code each pull request once it's merged will create a conflict with the following request in line. So, instead of merging the first one, it would be easier to merge the last one, then all changes are merged at the same time. However, I actually prefer it this way because then each significant change is recorded in the commit history as an individual thing, and I also get to review the code once more.

I don't think it will cause problems afterwards because I verify that in my system it merges cleanly, I just need to rebase the code each time a pull request is merged. So far only three remaining requests.
I am a bit more back into action now (big life changes ongoing..) so hopefully can help much better.
You got married?! Congratulations!
Last edited by vocx on Thu Jan 09, 2020 12:07 am, edited 1 time in total.
Always add the important information to your posts if you need help. Also see Tutorials and Video tutorials.
To support the documentation effort, and code development, your donation is appreciated: liberapay.com/FreeCAD.
carlopav
Veteran
Posts: 2062
Joined: Mon Dec 31, 2018 1:49 pm
Location: Venice, Italy

Re: [Discussion] Splitting Draft tools into their own modules

Post by carlopav »

Thx @vocx for the good work in putting order in the code, hope this will be merged soon!
yorik wrote: Wed Jan 08, 2020 3:29 pm I am a bit more back into action now (big life changes ongoing..) so hopefully can help much better.
Aannd well, congratulations for the big good changes Yorik!!
follow my experiments on BIM modelling for architecture design
User avatar
yorik
Founder
Posts: 13640
Joined: Tue Feb 17, 2009 9:16 pm
Location: Brussels
Contact:

Re: [Discussion] Splitting Draft tools into their own modules

Post by yorik »

vocx wrote: Wed Jan 08, 2020 9:32 pm You got married?! Congratulations!
Not that far :)
But I'm more or less switching continents once again...
User avatar
bernd
Veteran
Posts: 12849
Joined: Sun Sep 08, 2013 8:07 pm
Location: Zürich, Switzerland
Contact:

Re: [Discussion] Splitting Draft tools into their own modules

Post by bernd »

which one will you go to?
User avatar
yorik
Founder
Posts: 13640
Joined: Tue Feb 17, 2009 9:16 pm
Location: Brussels
Contact:

Re: [Discussion] Splitting Draft tools into their own modules

Post by yorik »

the belgian continent i guess :)
User avatar
bernd
Veteran
Posts: 12849
Joined: Sun Sep 08, 2013 8:07 pm
Location: Zürich, Switzerland
Contact:

Re: [Discussion] Splitting Draft tools into their own modules

Post by bernd »

wow we may meet than more than just once in years ...

For private or buissiness reasons?
vocx
Veteran
Posts: 5197
Joined: Thu Oct 18, 2018 9:18 pm

Re: [Discussion] Splitting Draft tools into their own modules

Post by vocx »

To continue the discussion about the previous long post in this thread, and the pull requests indicated in this post, I present the following image that more or less has my idea of how the workbench is currently structured.

We should do our best to move all graphical commands into their own modules, which means reducing DraftGui.py and DraftTools.py to just importing those modules. In the same manner, FeaturePython objects and their view providers should be moved to their own modules; we should be extra careful about this migration because we risk breaking compatibility with older files if new versions of the program cannot open the old classes. We should use the tricks mentioned in Migrating and upgrading old scripted objects.

This image is not entirely accurate, and doesn't show all relationships of the modules. It is meant to be an overview of what we have and what we want to achieve with the re-organization of the code.
Draft_architecture.png
Draft_architecture.png (234.44 KiB) Viewed 1469 times
Draft_architecture.svg
(103.45 KiB) Downloaded 52 times
Always add the important information to your posts if you need help. Also see Tutorials and Video tutorials.
To support the documentation effort, and code development, your donation is appreciated: liberapay.com/FreeCAD.
vocx
Veteran
Posts: 5197
Joined: Thu Oct 18, 2018 9:18 pm

Re: [Discussion] Splitting Draft tools into their own modules

Post by vocx »

Some more updates. These requests need to be merged in sequence, one after the other. But first, #3017, and all the previous requests need to be merged.

* Pull request #3091: re-organization of Draft_Snap commands. They are moved to individual modules to make DraftTools.py smaller. We also avoid star imports, like from DraftSnap import *. We don't want this in any file.
* Pull request #3092: re-organization of Draft Trackers classes. They are moved to another module, so that the root directory has less files. We also avoid star imports.
* The previous request needs changes in BIM as well; pull #38 in BIM.

These are simpler and more straight-forward changes:
* Pull request #3093: changing a few imports both in Draft and Arch, to import from the small utility modules instead of DraftGui and DraftTools. We want to avoid importing from these modules because they are very big and heavy.
* Pull request #3094: moved the Draft_SelectPlane command.
* Pull request #3095: moved the ShapeStringTaskPanel to a separate module to reduce the size of DraftGui.py.
* Pull request #3096: moved the ScaleTaskPanel to a separate module to reduce the size of DraftGui.py.
* Pull request #3097: moved the Draft_Edit command.
* The previous request will conflict with carlopav's request, #3023, so one of them has to be rebased depending on which is merged first.
Always add the important information to your posts if you need help. Also see Tutorials and Video tutorials.
To support the documentation effort, and code development, your donation is appreciated: liberapay.com/FreeCAD.
Post Reply