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

Refactor topology.py #788

Open
gumyr opened this issue Nov 16, 2024 · 0 comments
Open

Refactor topology.py #788

gumyr opened this issue Nov 16, 2024 · 0 comments
Assignees
Labels
architecture review Discussion related to architecture changes prior to 1st release

Comments

@gumyr
Copy link
Owner

gumyr commented Nov 16, 2024

The topology.py module is approaching 10,000 lines as it contains all of the code defining the entire topological layer of build123d objects. The dependencies between topological objects makes separating this module very difficult due to circular imports. The content of topology.py is expected to grow as new functionality is added so this problem highlights significant technical debt. Unfortunately, this refactor may break backwards compatibility so it's critical that this is addressed prior to release 1.0.0.

The following refactor options have been proposed:

  1. Split based on class with all of the existing methods remaining.
  2. Split based on class while converting problematic methods to functions.
  3. Split based on dimension (e.g. 0: Vertex, 1: Edge & Wire, 2: Face & Shell, etc.) while maintaining existing methods.

Imports would occur: Shape->Vertex->Edge->Wire->Face->Shell->Solid->Compound for the class based options or Shape->Vertex->Edge&Wire->Face&Shell->Solid->Compound for the dimension based option(s).

Given the tight coupling of objects with the same dimensions and the use of common Mixin classes, option 3 seems the most feasible based on prototyping and results in this sized split:

  • 681 topology/compound.py
  • 2416 topology/edge_wire.py
  • 1456 topology/face_shell.py
  • 493 topology/shape_list.py
  • 2134 topology/shape.py
  • 1229 topology/solid.py
  • 393 topology/utils.py
  • 197 topology/vertex.py
  • 8999 of the 9246 lines in topology.py (a few classes weren't converted in the prototype)

Many issues need to be resolved for this refactor to be successful.

@gumyr gumyr added the architecture review Discussion related to architecture changes prior to 1st release label Nov 16, 2024
@gumyr gumyr added this to the Gating Release 1.0.0 milestone Nov 16, 2024
@gumyr gumyr self-assigned this Nov 16, 2024
gumyr added a commit that referenced this issue Nov 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
architecture review Discussion related to architecture changes prior to 1st release
Projects
None yet
Development

No branches or pull requests

1 participant