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

Update documentation for pack function #641

Merged
merged 2 commits into from
Jun 12, 2024

Conversation

roman-dvorak
Copy link
Contributor

Hi,
As I mentioned, I tried to expand the documentation by including documentation of the pack function.

@gumyr
Copy link
Owner

gumyr commented Jun 11, 2024

One minor typo: "If you place the arranged objects into a Compose, you can easily determine their bounding box and check whether the objects fit on your print bed." - it should be Compound not Compose.

What do you think about putting most of this documentation into the pack docstring instead of the documentation? autofunction can extract all of the text from the docstring when building the docs and this extra info will be there for anyone who is reading the code directly or using an IDE to view the function. The ExportDXF has example code in the docsting as follows:

    Example:

        .. code-block:: python

            exporter = ExportDXF(unit=Unit.MM, line_weight=0.5)
            exporter.add_layer("Layer 1", color=ColorIndex.RED, line_type=LineType.DASHED)
            exporter.add_shape(shape_object, layer="Layer 1")
            exporter.write("output.dxf")

Lastly, I'm not sure that the description of _pack2d is required in the docs as users don't need this info. Maybe just as a comment in the code?

Thanks for improving the docs!

@gumyr gumyr merged commit 0c4cfb9 into gumyr:dev Jun 12, 2024
11 checks passed
@gumyr
Copy link
Owner

gumyr commented Jun 12, 2024

Thanks!

@jdegenstein
Copy link
Collaborator

@roman-dvorak It looks like the SVG assets here https://build123d.readthedocs.io/en/latest/assemblies.html#pack.pack were never added and as a result the links are broken. Could you open another PR and add them?

@roman-dvorak
Copy link
Contributor Author

Thank you for notification. I'm going to check it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants