Doc review - Githubissues

I have read more or less all the doc from top to bottom and here is my summed up review :

The categories on the left seems non ordered and some categories aren't well defined. I would propose to think of a better structure and maybe nest another level of categories, to separate better the different concepts. For example there is a BuildLine category at the same level of Introduction or Installation. Something like Builders -> BuildPart , BuildSkectch, BuildLine as sub levels would probably make more sense.
The use of the context of build123d are mentionned but I din't found any mention either in Introduction or Concepts of the fact that build123d does some magic internally with the contexts. A proper, magicless use of a Python context would be :
```
with BuildPart() as partbuilder:
my_box = Box(partbuilder, 10,10,10)
```
But instead b123d magicly understand that the Box class should interact with the partbuilder somehow.
```
with BuildPart() as partbuilder:
my_box = Box(10,10,10)
```
An advanced section with explanation of the internals (what is a Shape, Face, Compound and how build123d works with them could be interesting.
There is references of the internals of cadquery. It could makes sense when b123d was expected to be used by CQ users, now that it's less and less the case, removing those references could help clarify the situation (and help readers by not get tangled with two libs)
The BuildLine section while not positionned where it should is amazingly well explained ! We definitly need the same kind of thing for all the builders (and maybe the other contexts). If doing this for everything. Refactoring the Concepts section by just providing quick overview of all the lib capabilities and then leading the reader to these specific well enclosed doc sections.
Each code snippet should be associated with at least it's result image, and if possible (and when it make sense) the 3D view like CQ does. (Since CQ does it, it's possible but probably hard to integrate and might slow down the pages too much)
Tutorial section should be raised upper and be just avec the "quick" concept intro. No one will want to read all the detailed docs of the contexts before looking at the tutorials.
We have no mention of center on discord we had long discussions about centering and the mess it could be, in an Advanced section talking about this in more details could be interesting.

Tutorial specific review (I write them as I read them) :

Overall remarks

It would be nice if we could make all references of classes a link to the docs. (It would be even best with the hover extension so one could see what is a GridLocation while browsing the example without leaving the page)
Is it possible to add a "copy" button to allow the reader to copy the snippet and paste it in its working env ? (adding the from build123d import * line should probably be added to all snippet in this case) Examples specific remarks:
You use +ve and -ve for referring a normal direction the convention is more + or - Vz or vz isn't it ?
The complexity raise seems fine to me while a bit messy on the last examples, but I'm probably not the best suited to have a valid opinion on that since I know how things works.
The general tutorials are more example than tutorials, maybe the two concepts should be separated. (A tutorial looks more like the selector one, when we drive the user from nothing to a finished part while explaining the process along the way)
Example 4 MakeFace is incorporated without a mention, it's very not clear what this does, explaining that BuildSketch works only on closed faces and that it needs to extract a closed face out of the previous BuildLine context is mandatory imho.
Example 5 More explanation of what are locations (and that they are more than just points) would be suited here. We have to remember that people will usually start by reading examples rather than the full long theory, so explaining the same thing several time is not a problem, rather wanted actually. Showing that we get the same result if we pass the two locations to the same Locations context should be added. EDIT After having read example 6 which shows list of points, maybe it's better to leave only 1 location in Exemple 5 and not take care of my previous argument.
Example 9 _groupby(Axis.Z) returns a list of lists that is grouped by their z-position. A list of what ? Explains that we will use group_by to get the edges we want to fillet/chamfer. We have the answer in the snippet but in the text above it's not clear what we want to do.
Example 11 Not determined yet, but if we can't make a plane on a non planar face, explicitly tell here that the face must be planar. Maybe adding a note here explaining that one has to understand that calling Extrude will always create a shape that will be combined with the base part. Going - distance and Mode.Add or + dist and Mode.SUBTRACT can give unpexpected results.
Example 12 Operators @ and % are used without prior mention, it is explained earlier in the docs but a reminder would be nice.
Example 14 The operators are introduced but already used in example 12. Swap them or remove the example 12 use of them. Explain that the sweep sketch must lie on the path (I think it is ?). I'd advice offsetting everything to show it and not let everything at (0,0,0) which is actually a specfial case.
Example 15 Already a use of Mirror in example 8.
Example 16 @gumyr Does mirror copy the object location but references the main obj or do a hard copy of everything?
Example 20 The orientation of the cylinder doesn't allow to correctly appreciate the offset of the plane, reduce cylinder size and augment offset distance so we can see that the cylinder doesnt "touch" the rectangle anymore.
Example 24 Maybe explain what Loft do internally ? (Takes all the sketch in the context, treat them as section and interpolates between them) with a note that if sketch are not offsetted together it will probably fail.
Example 25 Tell the reader that Offset algorithm is very brittle and self intersecting edges will most likely break it.
Example 26 Even more true for 3D offsets.
Example 28 This was hard to understand but with some debugging it actually makes sense. Really complex use case but a good example.
Example 29 The real bottle of OCC has threads at the top😛
Example 30 This is not really complicated, it should not be that lot on the list
With all the examples, it would be good (if not already the case) to make test out of it. It's nice for extending test cases and also to ensure that they are never broken. (Nothing more frustrating than a non working example).

Joint tutorial :

The Hinge class inherits Compound maybe this will need to be update to BasePartClass in the future (The joint tutorial felt like an hour to read and I had enough for now, but it seems pretty well explained) I might go back and edit this issue in the future.

@jdegenstein It has been a long time but here is the review of your examples

First comment is that I totally agree that "General Tutorials" should be moved to a new category "Examples" and possibly be named "Introductory Examples" or something like that. I also agree that these (examples/tutorials) should be higher in the sidebar list, as most new users just want to get their feet wet with practical examples.

Thanks for reviewing the docs.

The categories on the left seems non ordered and some categories aren't well defined. I would propose to think of a better structure and maybe nest another level of categories, to separate better the different concepts. For example there is a BuildLine category at the same level of Introduction or Installation. Something like Builders -> BuildPart , BuildSkectch, BuildLine as sub levels would probably make more sense.

I intended to have a BuildLine, BuildSketch and BuildPart section at the top level - just got to the BuildLine one so far. I'm happy with a Builders -> BuildLine, BuildSketch, BuildPart organization as well.

The use of the context of build123d are mentionned but I din't found any mention either in Introduction or Concepts of the fact that build123d does some magic internally with the contexts. A proper, magicless use of a Python context would be : with BuildPart() as partbuilder: my_box = Box(partbuilder, 10,10,10) But instead b123d magicly understand that the Box class should interact with the partbuilder somehow.
with BuildPart() as partbuilder:
my_box = Box(10,10,10)

This is a nice description and needs to be added. Using redirect_stdout as an example might also help.

An advanced section with explanation of the internals (what is a Shape, Face, Compound and how build123d works with them could be interesting.

Agreed.

There is references of the internals of cadquery. It could makes sense when b123d was expected to be used by CQ users, now that it's less and less the case, removing those references could help clarify the situation (and help readers by not get tangled with two libs)

Agree, I've tried to remove most of them - I think Advantages is the only section them. Many users will come from cq so may just a short section in Introduction?

The BuildLine section while not positionned where it should is amazingly well explained ! We definitly need the same kind of thing for all the builders (and maybe the other contexts). If doing this for everything. Refactoring the Concepts section by just providing quick overview of all the lib capabilities and then leading the reader to these specific well enclosed doc sections.

Agreed

Each code snippet should be associated with at least it's result image, and if possible (and when it make sense) the 3D view like CQ does. (Since CQ does it, it's possible but probably hard to integrate and might slow down the pages too much)

There is some development required to get the sphinx view thingy going but I think @jdegenstein and @bernhard-42 have done most of the work that will be required.

Tutorial section should be raised upper and be just avec the "quick" concept intro. No one will want to read all the detailed docs of the contexts before looking at the tutorials.

Sounds good.

We have no mention of center on discord we had long discussions about centering and the mess it could be, in an Advanced section talking about this in more details could be interesting.

I could add an Advanced section that these types of things could be documented in. @bernhard-42 had a very comprehensive view of center is discord - it would be sad to loose it.

Tutorial specific review (I write them as I read them) : Overall remarks

It would be nice if we could make all references of classes a link to the docs. (It would be even best with the hover extension so one could see what is a GridLocation while browsing the example without leaving the page)

Agreed - just a bunch of copy paste.

Is it possible to add a "copy" button to allow the reader to copy the snippet and paste it in its working env ? (adding the from build123d import * line should probably be added to all snippet in this case)

Yes. https://sphinx-copybutton.readthedocs.io/en/latest/

Examples specific remarks: You use +ve and -ve for referring a normal direction the convention is more + or - Vz or vz isn't it ?

Should be changed.

The general tutorials are more example than tutorials, maybe the two concepts should be separated. (A tutorial looks more like the selector one, when we drive the user from nothing to a finished part while explaining the process along the way)

Sounds good.

So would the docs would be arranged like this:

Introduction --> Differences from CadQuery Installation Cheat Sheet Concepts Introductory Examples Tutorials Builders --> BuildLine --> BuildSketch --> BuildPart --> (BuildAssembly) Advanced --> (Assemblies) --> Custom Builder Objects & Operations --> Center --> Debugging / Logging --> CAD objects External Tools and Libraries Build API Reference Direct API Reference

I'm not 100% sure what to do about Assemblies - it seems a little out of place here. I've wondered about making a simple Builder for them which would therefore result in BuildAssembly in the Builders section.

I would put the cheat sheet lower. If we consider a top to bottom reading, Cheat sheet should be before External Tools and Libs or Build API Reference I think. This aside This structure sounds better to me.

Do you agree that Assemblies and Joints are tied together ? For me an assembly in build123d is a tree structure of parts linked together by joints.

On another note : Do you plan to add translations of the docs at some point ? You may know that I'm French and not all Frenchs knows how to read English, I could think of doing some translation but that's a lot of work which is maybe not really worth it. (If you are doing CAD with programming you usually know english to some extend)

I've restructured as discussed above. There are some empty sections that need to be filled out:

Build Sketch
Build Part
CAD object centers
Debugging & Logging

I've added the Sphinx copy addon but not Sphinx hover (https://sphinx-hoverxref.readthedocs.io/en/latest/) which will require some work. @jdegenstein, you were looking at this - did you make any progress?

Do you agree that Assemblies and Joints are tied together ? For me an assembly in build123d is a tree structure of parts linked together by joints.

Although I expect they will be used in an assembly context they don't have to be used that way. There is a tutorial but no specific section on Joints - I'm trying to decide if it should be part of assemblies or a separate section.

On another note : Do you plan to add translations of the docs at some point ? You may know that I'm French and not all Frenchs knows how to read English, I could think of doing some translation but that's a lot of work which is maybe not really worth it. (If you are doing CAD with programming you usually know english to some extend)

This type of localization is supported by Sphinx but it would a lot of work and there would need to be people willing to do the actual translations as, other than a little French, I'm uni-lingual. I'm thinking that all of the doc-strings within the code would be in English no matter what, but maybe there is a way to translate them too.

I just pasted the first two paragraphs of the build123d docs into https://deepl.com to translate to German. The result was really great, both from a correctness and readability perspective. Can't judge on French, but with tools like deepl.com I really ask myself, whether docs still need to be translated and maintained in several languages:

Build123d est un cadre de modélisation paramétrique et de représentation des limites (BREP) basé sur python pour la CAO 2D et 3D. Il est construit sur le noyau géométrique Open Cascade et permet la création de modèles complexes à l'aide d'une syntaxe python simple et intuitive. Build123d peut être utilisé pour créer des modèles pour l'impression 3D, l'usinage CNC, la découpe laser et d'autres processus de fabrication. Les modèles peuvent être exportés vers une grande variété d'outils de CAO populaires tels que FreeCAD et SolidWorks.

Build123d peut être considéré comme une évolution de CadQuery où l'API Fluent quelque peu restrictive (enchaînement de méthodes) est remplacée par des gestionnaires de contexte à état - par exemple avec des blocs - permettant ainsi d'utiliser toute la boîte à outils python : boucles for, références à des objets, tri et filtrage d'objets, etc.

Traduit avec www.DeepL.com/Translator (version gratuite)

Is that understandable from a native French perspective?

I didn't thought about translation tools but that's indeed totally acceptable. It confirms that doing manual translation is definitely useless from a time perspective. Let's forget about that topic

First off -- thanks for all the great feedback!

Example 4 MakeFace is incorporated without a mention, it's very not clear what this does, explaining that BuildSketch works only on closed faces and that it needs to extract a closed face out of the previous BuildLine context is mandatory imho.

Added a section about MakeFace and closed lines/faces

Example 5 More explanation of what are locations (and that they are more than just points) would be suited here. We have to remember that people will usually start by reading examples rather than the full long theory, so explaining the same thing several time is not a problem, rather wanted actually. Showing that we get the same result if we pass the two locations to the same Locations context should be added. EDIT After having read example 6 which shows list of points, maybe it's better to leave only 1 location in Exemple 5 and not take care of my previous argument.

I feel that explaining locations fully is important, but given that this is the first example involving them, I think we should save that description for a later example (possibly a new example).

Example 9 group_by(Axis.Z) returns a list of lists that is grouped by their z-position. A list of what ? Explains that we will use group_by to get the edges we want to fillet/chamfer. We have the answer in the snippet but in the text above it's not clear what we want to do.

I improved the explanation to include a mention that it is a list of lists of edges.

Example 11 Not determined yet, but if we can't make a plane on a non planar face, explicitly tell here that the face must be planar. Maybe adding a note here explaining that one has to understand that calling Extrude will always create a shape that will be combined with the base part. Going - distance and Mode.Add or + dist and Mode.SUBTRACT can give unpexpected results.

I agree and added more description of needing to use planar faces, and added a section explaining how Extrude can give unexpected results if the "direction" and mode are not properly set.

Example 12 Operators @ and % are used without prior mention, it is explained earlier in the docs but a reminder would be nice.

I agree, and have removed the use of the operators in example 12.

Example 14 The operators are introduced but already used in example 12. Swap them or remove the example 12 use of them. Explain that the sweep sketch must lie on the path (I think it is ?). I'd advice offsetting everything to show it and not let everything at (0,0,0) which is actually a specfial case.

I prefer to keep this example as-is and explore this subject in a second more advanced version. This is one of the first situations in which pending edges AND faces are applied so I want to keep it relatively simple. I did add a description of the sweep needing to be on the path.

Example 15 Already a use of Mirror in example 8.

I am planning to leave this as-is.

Example 20 The orientation of the cylinder doesn't allow to correctly appreciate the offset of the plane, reduce cylinder size and augment offset distance so we can see that the cylinder doesnt "touch" the rectangle anymore.

I agree, I added more separation and reduced the cylinder size. I also added more description.

Example 24 Maybe explain what Loft do internally ? (Takes all the sketch in the context, treat them as section and interpolates between them) with a note that if sketch are not offsetted together it will probably fail.

I agree, I added new notes to cover both points.

Example 25 Tell the reader that Offset algorithm is very brittle and self intersecting edges will most likely break it.

I will add a note to Ex 26 about this to cover both

Example 26 Even more true for 3D offsets.

Added note

Example 28 This was hard to understand but with some debugging it actually makes sense. Really complex use case but a good example.

Example 29 The real bottle of OCC has threads at the top😛

I just made the same object as the CQ example docs did. We can add threads when our resident thread expert ports his library!

Example 30 This is not really complicated, it should not be that lot on the list

I agree, it was this late on the list because it was broken during early preparation of these examples. I will move it up to example 23 in a future commit

Happy to have made constructive feedback. About locations I think, they need a section in the docs with Workplanes in the Concepts section and we will be fine

Also, forgot to mention that https://github.com/gumyr/build123d/pull/122 is related to the hover extension for the readthedocs page.

About locations I think, they need a section in the docs with Workplanes in the Concepts section and we will be fine

There already is a Location section with the Workplane description:

Is there anything else that needs to be done to the docs before closing this issue?

Aside from adding what's left blank I think it's good. I guess making review like that from time to time can be nice to keep track of the evolution

Although documentation is never perfect, it includes the points described above and much more.

gumyr / build123d

Doc review #118

Tutorial specific review (I write them as I read them) :

Joint tutorial :