-
Notifications
You must be signed in to change notification settings - Fork 299
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
text() docstring update #813
Conversation
* Remove following line from docstring (appears to be copy/paste error from extrude()) extrude always *adds* material to a part. * Swap order of :param distance: and :param fontsize: to match argument order * Mention fontsize is in model units (fix CadQuery#463) * distance: mention distance is for both extrude/cut * Update kind default in docstring as "regular" not "Normal" * Add missing fontPath arg description * Add examples of usage - Using an installed system font - Using a font at provided path (when font not installed on system) I tested with the free fonts from http://www.gust.org.pl/projects/e-foundry/tex-gyre - Cut text into solid * Change docstring to fix autodoc-typehints issue where the Parameters section is duplicated (see screenshot)
Codecov Report
@@ Coverage Diff @@
## master #813 +/- ##
==========================================
- Coverage 95.17% 95.13% -0.04%
==========================================
Files 32 32
Lines 7413 7489 +76
Branches 796 798 +2
==========================================
+ Hits 7055 7125 +70
- Misses 221 225 +4
- Partials 137 139 +2
Continue to review full report at Codecov.
|
@marcus7070 Do the examples in this PR fall under the discussions you've been having about keeping examples in the docs up-to-date? |
Hmmmm, that's an interesting thought @jmwright. You could do a basic smoke test by using pytest's doctest integration and changing the docstring to: Examples:
>>> cq.Workplane().text("CadQuery",5,1)
Specify the font (name), and kind to use an installed system font:
>>> cq.Workplane().text("CadQuery",5,1,font="Liberation Sans Narrow", kind="italic")
Specify fontPath to use a font from a given file:
>>> cq.Workplane().text("CadQuery",5,1, fontPath="/opt/fonts/texgyrecursor-bold.otf")
Cutting text into a solid:
>>> cq.Workplane().box(8,8,8).faces(">Z").workplane().text("Z", 5,-1.0) But I hate doing double duty (document and test) with docstrings like this because It's also of limited benefit since it's pretty rare a docstring goes out of date with the function, since they're right next to each other. So my vote is for no tests in docstrings. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The only important change is that :param self:
one, the rest are trivial stuff but I was suggesting changes anyway so I thought I'd throw them in.
Thanks for doing this @lorenzncode!
Co-authored-by: Marcus Boyd <[email protected]>
Co-authored-by: Marcus Boyd <[email protected]>
Co-authored-by: Marcus Boyd <[email protected]>
Co-authored-by: Marcus Boyd <[email protected]>
Co-authored-by: Marcus Boyd <[email protected]>
Co-authored-by: Marcus Boyd <[email protected]>
Co-authored-by: Marcus Boyd <[email protected]>
Co-authored-by: Marcus Boyd <[email protected]>
Co-authored-by: Marcus Boyd <[email protected]>
Co-authored-by: Marcus Boyd <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for doing this @lorenzncode, even if we didn't get everything done it's still an improvement.
Thanks @lorenzncode ! Shall I merge @marcus7070 @jmwright ? |
+1 to merge |
If this fix looks good, I can follow up with a similar docstring fix for the duplicate Parameters issue on other methods.
current
update: