gumyr / build123d

A python CAD programming library
Apache License 2.0
506 stars 85 forks source link

Use of `<source class>.to_<target class>` methods #155

Open gumyr opened 1 year ago

gumyr commented 1 year ago

There are many methods that convert from one class to another - for example Vector.to_vertex(). These methods don't seem as pythonic as using the target class' __init__ method to do the conversion - e.g. Vertex(v:Vector). They also couple the classes together which makes separation into multiple modules impossible (or at least very difficult).

I'm proposing converting all of the to_class methods over to class(). Is there any reason not to do this?

Jojain commented 1 year ago

I propose this : Splitting the topo class and the geom classes in 2 modules :

gumyr commented 1 year ago

Although I haven't pushed it yet, I have have two modules: geometry.py and topology.py. All the tests and examples pass but the documentation will need to be looked at. There was only a minor bit of messiness as Planes can be constructed from Faces which needed to be re-worked (with the same functionality).

jdegenstein commented 1 year ago

wow, that is awesome progress!

gumyr commented 1 year ago

The restructuring to geometry.py and topology.py is complete. Onto elimination of the other .to_ methods...

jdegenstein commented 9 months ago

Taking a quick look at the to_ methods, geometry.py has these (line no's on left):


234 | def to_tuple(self) -> tuple[float, float, float]:

417 | def to_pnt(self) -> gp_Pnt:
 
421 | def to_dir(self) -> gp_Dir:
 
584 | def to_plane(self) -> Plane:
 
845 | def to_align_offset(self, align: Tuple[float, float]) -> Tuple[float, float]:
 
949 | def to_tuple(self) -> Tuple[float, float, float, float]:
 
1282 | def to_axis(self) -> Axis:
 
1286 | def to_tuple(self) -> tuple[tuple[float, float, float], tuple[float, float, float]]:
 
2088 | def to_gp_ax2(self) -> gp_Ax2:
 
2136 | def to_local_coords(self, obj: Union[VectorLike, Any, BoundBox]):

topology.py has these (line no's on left):

2847 |  def to_splines(

2880 | def to_vtk_poly_data(
 
2933 | def to_arcs(self, tolerance: float = 1e-3) -> Face:
 
4270 | def to_wire(self) -> Wire:
 
5001 | def to_axis(self) -> Axis:
 
6554 | def to_tuple(self) -> tuple[float, float, float]:
 
6682 | def to_wire(self) -> Wire:

Lastly jupyter_tools.py has def to_vtkpoly_string(...

Anything I can do to help trim these down / refactor?

gumyr commented 9 months ago

How does this sound:

The sub-classes of Shape don't have init methods and should if the to_wire methods are to be replaced. A bit of work though.

snoyer commented 9 months ago

FWIW the built-in tuple() initializer takes an iterable so technically you could get rid of .to_tuple() by overriding __iter__...

@dataclass
class A:
    x: float
    y: float
    z: float

    def to_tuple(self):
        return self.x, self.y, self.z

    def __iter__(self):
        return iter((self.x, self.y, self.z))

a = A(1, 2, 3)
assert a.to_tuple() == tuple(a)

__iter__ also allows unpacking

x,y,z = a
assert tuple(a) == (x,y,z)

I won't risk claiming whether this is the way to go or not though

gumyr commented 9 months ago

Both Vector and Vertex are Iterables already, so:

print(tuple(Vertex(1, 2, 3)))
print(tuple(Vector(4, 5, 6)))

results in

(1.0, 2.0, 3.0)
(4.0, 5.0, 6.0)