Fix the doctests #1340

tomvanmele · 2024-04-24T22:56:02Z

What type of change is this?

Bug fix in a backwards-compatible manner.
New feature in a backwards-compatible manner.
Breaking change: bug fix or new feature that involve incompatible API changes.
Other (e.g. doc update, configuration, etc)

Checklist

Put an x in the boxes that apply. You can also fill these out after creating the PR. If you're unsure about any of them, don't hesitate to ask. We're here to help! This is simply a reminder of what we are going to look for before merging your code.

I added a line to the CHANGELOG.md file in the Unreleased section under the most fitting heading (e.g. Added, Changed, Removed).
I ran all tests on my computer and it's all green (i.e. invoke test).
I ran lint on my computer and there are no errors (i.e. invoke lint).
I added new functions/classes and made them available on a second-level import, e.g. compas.datastructures.Mesh.
I have added tests that prove my fix is effective or that my feature works.
I have added necessary documentation (if appropriate)

tomvanmele · 2024-04-24T23:07:11Z

not all objects have __str__
having to use print(...) in docstring examples to make sure they don't fail due to precision errors is annoying
would be good to have a template structure of some sorts, or at least a few "rules", for docstring examples

other things i have noticed

shapes have u and v even when only u makes sense. perhaps that should be changed
many attributes could be cached. would be good to have a system for that so that the cache gets invalidated when the parameters of an object are changed (could be a decorator for some attributes linked to an invalidate_cache method)
would be useful to further extend the basic infrastructure of with a bounding box attribute
point (and vector), line, polyline, polygon, polyhedron share the fact that they are defined explicitly by points. they therefore also don't have a frame. would be good to document this as such.
the parametric objects do have a frame, would be good to point this out as well in the docs
some intersections are still missing
an intersection class that combines all intersection calculations would be useful.
in general there should be a rule for binary operations between geometric entities about which entity is "left" and which is "right". for example the lower-dimensional object could always be on the left...

tomvanmele · 2024-04-24T23:07:56Z

@jf--- since i can't add you as reviewer... ping

tomvanmele · 2024-04-24T23:08:35Z

my mouse and keyboard don't want to connect anymore so i will take that as a sign that it is time to give up for today...

tomvanmele · 2024-04-25T06:53:25Z

all are fixed.
some had to be skipped because of some weird behaviour, others because of an actual bug. we can do a search for # doctes: +SKIP to investigate in more detail later...

tomvanmele · 2024-04-25T06:54:01Z

the tests are failing because of something weird going on on the github side

Licini · 2024-04-25T14:13:12Z

src/compas/data/encoders.py

+    >>> with open('point.json', 'r') as f:        # doctest: +SKIP
+    ...     point = json.load(f, cls=DataDecoder) # doctest: +SKIP


just out of curiosity why these needs to be skipped? So in doctest the read and writing to disk is not possible? If that's the case perhaps these kind of stuff can be loads and dumps instead

perhaps this works now actually. will try again...

Licini · 2024-04-25T14:18:18Z

src/compas/geometry/curves/hyperbola.py

-    >>> from compas_viewer import Viewer  # doctest: +SKIP
-    >>> viewer = Viewer()                    # doctest: +SKIP
-    >>> viewer.scene.add(line)                  # doctest: +SKIP
+    >>> from compas_viewer import Viewer          # doctest: +SKIP
+    >>> viewer = Viewer()                         # doctest: +SKIP
+    >>> viewer.scene.add(line)                    # doctest: +SKIP
    >>> viewer.scene.add(hyperbola)               # doctest: +SKIP
    >>> viewer.scene.add(hyperbola.frame)         # doctest: +SKIP
-    >>> viewer.show()                      # doctest: +SKIP
+    >>> viewer.show()                             # doctest: +SKIP


Maybe in future we should remove them from API reference of core? (After we clean up and improve the documents of viewer)

yes i agree. viewer stuff should not be in the docs. will already remove them...

Licini · 2024-04-25T14:20:41Z

src/compas/geometry/intersection.py

+    >>> from compas.geometry import Line          # doctest: +SKIP
+    >>> from compas.geometry import Intersection  # doctest: +SKIP
+    >>> a = Line([0, 0, 0], [2, 0, 0])            # doctest: +SKIP
+    >>> b = Line([1, 0, 0], [1, 1, 0])            # doctest: +SKIP
+    >>> intersection = Intersection()             # doctest: +SKIP
+    >>> intersection.line_line(a, b)              # doctest: +SKIP
+    >>> intersection.number_of_intersections      # doctest: +SKIP


plugin required for these ones?

no, this was more like an API test example. the class is not yet part of the public API...

Licini · 2024-04-25T14:24:31Z

src/compas/geometry/offset.py

@@ -138,7 +138,7 @@ def offset_polygon(polygon, distance, tol=None):
    >>> polygon = Polygon([(0.0, 0.0), (1.0, 0.0), (1.0, 1.0), (0.0, 1.0)])
    >>> offsetted_polygon = offset_polygon(polygon, 0.5)
    >>> offsetted_polygon
-    Polygon[[0.5, 0.5, 0.0], [0.5, 0.5, 0.0], [0.5, 0.5, 0.0], [0.5, 0.5, 0.0]]
+    [[0.5, 0.5, 0.0], [0.5, 0.5, 0.0], [0.5, 0.5, 0.0], [0.5, 0.5, 0.0]]


shouldn't this function return a Polygon object instead of list to align with the input?

no, the lower level function shouldn't. the version attached to the polygon class itself should...

Licini · 2024-04-25T14:28:43Z

src/compas/geometry/polyhedron.py

@@ -491,7 +491,7 @@ def from_convex_hull(cls, points):
        --------
        >>> from compas.geometry import Polyhedron
        >>> points = [[0, 0, 0], [1, 0, 0], [0, 1, 0]]
-        >>> p = Polyhedron.from_convex_hull(points)
+        >>> p = Polyhedron.from_convex_hull(points)  # doctest: +SKIP


It would be nice if we also have a way document the minimal plugins needed for certain functions

Licini

LGTM! Just some random comments!

jf--- · 2024-04-27T08:33:49Z

@tomvanmele this is exciting, is this PR tangential to #1340?

tomvanmele · 2024-04-27T10:37:04Z

which one do you mean? this one is #1340...

gonzalocasas · 2024-07-02T11:48:45Z

@tomvanmele there's quite a bunch of conflicts now, but it would be cool to merge this

tomvanmele requested review from gonzalocasas, chenkasirer and Licini April 24, 2024 23:07

Licini reviewed Apr 25, 2024

View reviewed changes

Licini approved these changes Apr 25, 2024

View reviewed changes

tomvanmele added 6 commits July 2, 2024 21:07

around 20 to go

dd28aa7

that's all folks

850fa7c

remove examples

594b0c9

use 'ed' version

2199251

correct print output

c1584f7

latest polyhedron implementations

a4b78c0

tomvanmele force-pushed the fix-doctests branch from 32e684d to a4b78c0 Compare July 2, 2024 19:22

log

bb5258e

tomvanmele merged commit 0d71a41 into main Jul 3, 2024
14 checks passed

yck011522 mentioned this pull request Jul 11, 2024

What happened to intersection_sphere_sphere #1381

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix the doctests #1340

Fix the doctests #1340

tomvanmele commented Apr 24, 2024

tomvanmele commented Apr 24, 2024

tomvanmele commented Apr 24, 2024

tomvanmele commented Apr 24, 2024

tomvanmele commented Apr 25, 2024

tomvanmele commented Apr 25, 2024

Licini Apr 25, 2024

tomvanmele Apr 25, 2024

Licini Apr 25, 2024

tomvanmele Apr 25, 2024

Licini Apr 25, 2024

tomvanmele Apr 25, 2024

Licini Apr 25, 2024

tomvanmele Apr 25, 2024

Licini Apr 29, 2024

Licini Apr 25, 2024

Licini left a comment

jf--- commented Apr 27, 2024

tomvanmele commented Apr 27, 2024

gonzalocasas commented Jul 2, 2024

		>>> with open('point.json', 'r') as f: # doctest: +SKIP
		... point = json.load(f, cls=DataDecoder) # doctest: +SKIP

Fix the doctests #1340

Fix the doctests #1340

Conversation

tomvanmele commented Apr 24, 2024

What type of change is this?

Checklist

tomvanmele commented Apr 24, 2024

tomvanmele commented Apr 24, 2024

tomvanmele commented Apr 24, 2024

tomvanmele commented Apr 25, 2024

tomvanmele commented Apr 25, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Licini left a comment

Choose a reason for hiding this comment

jf--- commented Apr 27, 2024

tomvanmele commented Apr 27, 2024

gonzalocasas commented Jul 2, 2024