Collections namespace, clean PolygonGeometry class #351
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #351 +/- ##
=======================================
Coverage 87.65% 87.66%
=======================================
Files 95 95
Lines 5670 5674 +4
=======================================
+ Hits 4970 4974 +4
Misses 700 700 ☔ View full report in Codecov by Sentry. |
src/specklepy/objects/other.py
Outdated
| name: Optional[str] = None, | ||
| elements: Optional[List[Base]] = None, | ||
| units: Optional[str] = None, | ||
| ) -> None: |
There was a problem hiding this comment.
possibly a breaking change for the constructors that were passing collectionType
P.S. removed
| units: Optional[str] = None, | ||
| boundary: Optional[Polyline] = None, | ||
| voids: Optional[List[Polyline]] = None, | ||
| ) -> None: |
There was a problem hiding this comment.
ideally all these should be Required, but we need to have an option for an empty constructor because of the serializer design. On Receive, objects are being initialized from JSON with empty constructors first
| class Collection( | ||
| Base, speckle_type="Speckle.Core.Models.Collection", detachable={"elements"} | ||
| ): | ||
| name: Optional[str] = None | ||
| collectionType: Optional[str] = None | ||
| elements: Optional[List[Base]] = None | ||
|
|
||
|
|
||
| class Collection( # noqa: F811 |
There was a problem hiding this comment.
Will two classes with the same name work? is that what this F811 suppression is for?
I'm a bit worried that this may break the Blender connector...
There was a problem hiding this comment.
@JR-Morgan oops didn't see your review 🙈
yep, I tested, and it is the same way it works with VectorLayer for example. But I am open to any other possibility, it's just I haven't found a way to safely change one speckle_type to another
No description provided.