Implement `AbstractPath` and related API changes to support multiple path types #3535

nickbianco · 2023-08-26T00:38:10Z

Fixes issue #3388

Brief summary of changes

This PR implements AbstractPath, a new base class from which all path-like objects can derive from, including GeometryPath. Specifically, derived objects will need to implement virtual methods that provide the path length, lengthening speed, moment arms, body and mobility forces, and visualization properties.

My current attempt at this implementation led to a few API changes:

Muscles and other path-dependent objects can no longer have convenience methods for adding path points (e.g., Muscle::addNewPathPoint), since other path types might not use PathPoints at all.
Classes that had a GeometryPath property (i.e., PathActuator, PathSpring, Ligament, and Blankevoort1991Ligament), now have an AbstractPath property, which has been moved to protected (TODO: move to private?).
All methods updGeometryPath and getGeometryPath have been changed to updPath and getPath, which return AbstractPath references.
Templatized signatures for updPath and getPath for returning concrete objects (e.g., GeometryPath) have been added.
I've introduced initGeometryPath, which is a safe method for overwriting the path-based object's current path with a GeometryPath. While the current default value for the path properties is GeometryPath, future methods can be introduced to initialize paths of different types (e.g., initFunctionBasedPath). I based this approach on how we initialize different solvers in MocoStudy.

These changes led to a lot of changed files, but most of these files are tests or examples with models with very simple paths (e.g., a single origin and insertion point).

Testing I've completed

Ran all the existing wrapping tests. I'm happy to expand or add new tests, especially for the interface changes.

Looking for feedback on...

Any feedback about the API changes and how they might affect users. This will obviously affect the GUI and OpenSim Creator, but my assumption is that most users do not interact much with GeometryPath directly and the effects of the API changes will be mostly internal. However, I am prepared to be totally wrong about that and I'm happy to make changes that best suit everyone.

CHANGELOG.md (choose one)

updated (but, TODO API changes).

This change is

nickbianco · 2023-08-26T03:46:39Z

Tests are failing because I haven't updated the bindings -- will do that next week.

nickbianco · 2023-08-28T22:22:43Z

@aymanhab, I made some changes to the Java binding code to get everything to compile for now. We can discuss together what changes make the most sense for the GUI.

Working on the other test failures now.

adamkewley

Mostly looks fine but I would strongly consider supplying [[deprecated]] backwards-compatible methods, rather than entirely removing the old API, where those methods runtime-downcast-and-check that the implementation is point-based, throwing otherwise. By doing that, you'll drastically reduce this diff while making rollout of the change easier for downstream scripting users.

There's also some shortcuts w.r.t. copying paths in the code. I'd recommend carefully reading where I've mentioned a change changes intent/behavior. The new change may compile, but it is just installing a later issue that someone else has to dig up and figure out what's going on (e.g. "why does my UI crash if I load a model (with FBPs)", "why can't I convert these FBP-based muscles to DeGrootFregly muscles", etc.)

Applications/Scale/test/testScale.cpp

Bindings/Java/OpenSimJNI/OpenSimContext.cpp

Bindings/Java/OpenSimJNI/Test/testContext.cpp

Bindings/Java/tests/TestBasics.java

OpenSim/Simulation/Model/AbstractPath.h

adamkewley · 2023-08-29T09:17:03Z

OpenSim/Simulation/Model/Blankevoort1991Ligament.cpp

@@ -62,6 +44,11 @@ Blankevoort1991Ligament::Blankevoort1991Ligament(
    set_slack_length(slack_length);
 }

+GeometryPath& Blankevoort1991Ligament::initGeometryPath() {


init methods on classes are usually a code smell - unless you're writing like a game developer with exceptions turned off or something.

I'm trying to find a way to support creating new objects that own paths (of any type) that is scripting friendly. I'm not sure that this is the best approach. Another approach could be the pointer adoptee approach used elsewhere in OpenSim (although I'm not sure how to update the path property in that case).

OpenSim/Simulation/Model/GeometryPath.cpp

OpenSim/Simulation/Model/PathActuator.h

adamkewley · 2023-08-29T09:36:06Z

OpenSim/Tests/Wrapping/testWrapCylinder.cpp

@@ -451,8 +453,9 @@ namespace {

        // Trigger computing the wrapping path (realizing the state will not).
        model.getComponent<PathSpring>("spring").getLength(state);
-        const WrapResult wrapResult = model.getComponent<PathSpring>("spring")
-                .getGeometryPath()
+        const auto& path = dynamic_cast<const GeometryPath&>(


getPath<T>

adamkewley

Looks like good changes.

The only thing I noticed was the new method on Muscle, why's it there?

OpenSim/Simulation/Model/Muscle.h

adamkewley · 2023-08-30T07:57:20Z

OpenSim/Simulation/Model/PathActuator.h

@@ -66,17 +66,35 @@ class OSIMSIMULATION_API PathActuator : public ScalarActuator {
    // GET AND SET
    //--------------------------------------------------------------------------
    // Path
+    bool hasPath() const override { return true;};


nit: space after ;

aymanhab

Reviewed 1 of 62 files at r1, 1 of 43 files at r3, 1 of 16 files at r4.
Reviewable status: 2 of 67 files reviewed, 20 unresolved discussions (waiting on @adamkewley and @nickbianco)

CHANGELOG.md line 11 at r2 (raw file):

Previously, nickbianco (Nicholas Bianco) wrote…

Per your suggestions, I'm opting to revert API changes since supporting getGeometryPath and updGeometryPath is straight-forward anyway. But I agree, in the end, I'd prefer minimal API changes for those reasons.

👍

OpenSim/Simulation/Model/AbstractPath.h line 102 at r4 (raw file):

     */
    virtual SimTK::Vec3 getColor(const SimTK::State& s) const = 0;

I feel most of the appearance and color stuff doesn't belong here and can be moved to GeometryPath but hard to tell looking at this class if that would cause other problems

aymanhab

Reviewed 2 of 16 files at r4.
Reviewable status: 4 of 67 files reviewed, 20 unresolved discussions (waiting on @adamkewley and @nickbianco)

Bindings/Java/OpenSimJNI/OpenSimContext.cpp line 128 at r2 (raw file):

Previously, nickbianco (Nicholas Bianco) wrote…

Agreed. Reverting these changes for now since I've added back getGeometryPath(). @aymanhab and I can discuss what makes sense for a refactor since (I believe) this file is GUI-specific.

Indeed it is GUI specific, and some methods may not be used any more or can be easily refactored once the API settles

Bindings/Java/OpenSimJNI/OpenSimContext.cpp line 136 at r2 (raw file):

Previously, nickbianco (Nicholas Bianco) wrote…

Similar to above, reverting these changes since I've reverted the updGeometryPath change. Will discuss with @aymanhab what makes sense for a refactor here.

This function seems to be unused and can be removed when we revisit.

Bindings/Java/OpenSimJNI/Test/testContext.cpp line 0 at r2 (raw file):

Previously, adamkewley (Adam Kewley) wrote…

(Reviewable was unable to map this GitHub inline comment thread to the right spot — sorry!)

Is there any harm in keeping updGeometryPath() as an alias for updPath<GeometryPath>?

By removing addNewPathPoint, updGeometryPath, getGeometryPath, etc. you're publishing a correct(er) API, yes, but any downstream MATLAB/python scripts are going to be broken - even though there's no logical reason for them to break if they are operating on point-based paths (with legacy scripts, it is likely that they will be).

Prefer keeping any old API methods with a [[deprecated]] marking and have those legacy methods runtime-check the path type (in many cases, the paths will be point-based - for now). Maybe after sitting on it for a while the [[deprecated]] stuff can be dropped as part of a larger OpenSim5 rollout.

I wouldn't recommend breaking any existing OpenSim4 APIs. The vast majority of OpenSim's research userbase are using MATLAB/python, where they won't see the breakage coming and won't know why their scripts no longer work.

💯

adamkewley

Had another pass

My main concern is adding those if (dynamic_cast<GeometryPath>...) else { THROW } blocks. The reason why is because they're the type of thing that will be buried in the codebase and there will be no warning that there's an issue until a user reports it (e.g. it's unlikely that developers will proactively poke around Blankevoort etc. looking for downcasts when adding a new path abstraction).

Even if the solution is (e.g.) to have a copy-assignment method that throws if typeof(lhs) != typeof(rhs) then at least the explosion is closer to where people are likely to be looking when working on AbstractPath/GeometryPath (rather than it being buried in a particular implementation somewhere)

adamkewley · 2023-08-31T07:30:25Z

OpenSim/Actuators/DeGrooteFregly2016Muscle.cpp

-                        .connect(pathPointSet.get(ipp)
-                                         .getSocket(socketName)
-                                         .getConnecteeAsObject());
+        // Copy the muscle's path.


I'd tackle copying centrally in this PR rather than leaving an else as a future exercise: there's a high chance it'll be forgotten and undetected until a user runs into it at runtime (e.g. by using DeGrooteFregly2016Muscle with a non-GeometryPath)

Plus, if copy assignment of paths is something that's in downstream code (e.g. as it is here) then, architecturally speaking, it makes sense that copying is tackled centrally and code is ported accordingly.

OpenSim/Actuators/DeGrooteFregly2016Muscle.cpp

adamkewley · 2023-08-31T07:50:05Z

OpenSim/Actuators/ModelFactory.cpp

-                        .connect(pathPointSet.get(ip)
-                                .getSocket(socketName)
-                                .getConnecteeAsObject());
+        // Copy the muscle's path.


Don't do this. it's a bad idea for a large C++ codebase to start having many if (downcastFailed) { throw ...; } code, because it makes maintaining the codebase long-term much harder. The compiler won't be able to help developers when new abstractions are added, for example, and implementors will be forced to look for all of these dynamic_casts that you are bodging in.

Either:

Make the abstraction have enough pieces to perform the replacement virtually (required, if you plan on having external code implement new AbstractPaths independently of OpenSim)

Add a visitor pattern to the abstraction, where the visitor pattern double-dispatches to a known set of path implementations (constrains downstream implementations to a limited set, but adds static checks that will trigger when new paths are added)

Then you can do what you're doing within the framework of those solutions, but only if the concrete implementation has no feasible way of working without points (e.g. the visitor would throw for non-point-based paths with "there's no way of doing this operation with a function-based path - sorry not sorry" or similar).

OpenSim/Simulation/Model/Ligament.h

OpenSim/Simulation/Model/PathSpring.h

nickbianco · 2023-08-31T23:09:11Z

@adamkewley, thanks a bunch for the quick reviews across lots of changing files! Regarding the path copying utilities, I'm seeing your point clearer now. In this pass, I implemented a new virtual method AbstractPath::copyFrom() for handling copying of path properties. This still requires dynamic_casts, but they're now contained in the concrete implementations rather than hidden in utilities somewhere in OpenSim. This seemed simplest to me, but let me know if you think it's sufficient.

@aymanhab, now would be a good time to jump in for a full review.

Some thoughts I have about the current state of the PR:

@aymanhab, I saw your comment about moving the visualization methods to GeometryPath, but doing so would necessitate changes in extendReallizeDynamics() in both Ligament and PathActuator. We could change these, but it would require checking types inside these methods which might not be ideal. Having virtual methods like setColor() that aren't used by some derived classes (e.g., FunctionBasedPath) seems like bad design, but if it doesn't break anything downstream then maybe it's fine.
Ligament and Blankevoort1991Ligament both now have getters and setters for modifying the path property, whereas before the unnamed GeometryPath property was modifiable through auto-generated methods from OpenSim_DECLARE_PROPERTY (e.g., upd_GeometryPath). Therefore, the current changes are breaking for anyone who is using the auto-generated methods for these classes. Two solutions to this (neither of which I love) are: 1) revert back to the unnamed GeometryPath property for these classes (since ligaments usually don't need wrapping objects, as @aymanhab pointed out) or 2) implement deprecated methods that cover all of the auto-generated methods that were removed. What do you guys think?
Another API change: Force::hasGeometryPath() is now Force::hasPath(). Should we support backwards compatibility for hasGeometryPath() somehow (e.g., grab the property, see if a path exists, try a dynamic_cast, etc)?

aymanhab

@nickbianco I'd agree to avoid breaking changes to Ligaments, with that option 1 seems more straightforward.
I didn't find any usage of the method hasGeometryPath() in opensim-core, it's used by the GUI for special handling of visualization of GeometryPaths and as such should return true only when there're visuals to display/update (ties into the first comment) so we'll need to reimplement with dynamic_cast now or later, or find an alternative.

Reviewed 1 of 62 files at r1, 1 of 6 files at r8.
Reviewable status: 6 of 70 files reviewed, 23 unresolved discussions (waiting on @adamkewley and @nickbianco)

OpenSim/Analyses/MuscleAnalysis.h line 32 at r8 (raw file):

//=============================================================================
#include <OpenSim/Common/PropertyStrArray.h>
#include <OpenSim/Simulation/Model/Analysis.h>

Any reason this include is needed here, when the previous version didn't?

adamkewley

👍 all very good: thank you for enduring my feedback and making some nice changes.

Additional (very optional) suggestions:

For copying things, because AbstractPath supports .clone(), you may be able to clone the source AbstractPath into a new pointer and then assign that pointer to the relevant property etc. in the target datastructure. The benefit of that approach is that it would work for any path implementation, whereas copy assignment (i.e. copyFrom) is limited to only allowing T = T const& expressions, which is why the downcast is necessary. Not your fault (the original code is copy assignment), but might be an idea.
If you need copyFrom (e.g. because doing it at a higher level with clone is a no-go) then it might be better named as assign, to matchAbstractProperty::assign and because it's a common idiom (e.g. std::vector<T>::assign)

Again, these are very optional suggestions (observations, even) - take them or leave them

OpenSim/Actuators/DeGrooteFregly2016Muscle.cpp

OpenSim/Actuators/ModelFactory.cpp

OpenSim/Actuators/DeGrooteFregly2016Muscle.cpp

OpenSim/Simulation/Model/AbstractPath.h

OpenSim/Simulation/Model/GeometryPath.cpp

nickbianco · 2023-09-01T18:07:03Z

Thanks @adamkewley and @aymanhab!

A few more thoughts:

For copying things, because AbstractPath supports .clone(), you may be able to clone the source AbstractPath into a new pointer and then assign that pointer to the relevant property etc. in the target datastructure.

@adamkewley, I also considered this and like the idea of it, but couldn't fully wrap my head around it. Would default implementations of Object::clone() provided by OpenSim_OBJECT_CONCRETE_DEFS need to be skipped over (e.g., like Model does)? Or would you just keep the assign method and instead give it a pointer from the source object's clone()? Maybe this is a bit superfluous, but I'm inclined to do everything the "right way" now to save pain later since I'm going to be working very closely with these classes.

I'd agree to avoid breaking changes to Ligaments, with that option 1 seems more straightforward.

@aymanhab, I am tempted to take the simplest path here, but it feels wrong to not support the new wrapping abstraction for half of the classes that own path objects. After a quick look, I did find a breaking change in @clnsmith's opensim-jam project. Would it be so bad to write deprecated methods, even just for upd_GeometryPath and get_GeometryPath, since these are likely the most used? I don't want users who have models with many ligaments to miss the opportunity for a potentially faster solution. Personally, I would not want to be forced to use different path types simply because it's not supported for some of the objects in my model (but again, that is a personal preference and not a workflow issue).

I didn't find any usage of the method hasGeometryPath() in opensim-core, it's used by the GUI for special handling of visualization of GeometryPaths and as such should return true only when there're visuals to display/update (ties into the first comment) so we'll need to reimplement with dynamic_cast now or later, or find an alternative.

I see, so setting hasPath to true for having any path is changing the intended functionality. Let me draft something and see what you think.

adamkewley · 2023-09-01T18:14:08Z

Would default implementations of Object::clone() provided by OpenSim_OBJECT_CONCRETE_DEFS need to be skipped over (e.g., like Model does)?

You want the clone's implementation to be provided via the CONCRETE_DEFS so that a base-er class's virtual version of it can be implemented by it.

For where the pointer needs to go, I imagine that any OpenSim collection of abstract objects must be storing them as pointers (they're abstract, so there is no way for the compiler to know how much space they need), so the "better" (clone-based) approach would need to install the cloned pointer into the underlying pointer holder/array that's used for abstract properties (if that makes sense) - I believe OpenSim::ObjectProperty has assign methods specifically for doing the clone-then-overwrite operation via pointers.

nickbianco · 2023-09-01T19:01:45Z

@adamkewley, I've implemented your suggestions above for now. I'm thinking over your recent comments and taking a look ObjectProperty. This is getting into OpenSim territory (and, let's be honest, C++ concepts) I'm not as familiar with. Maybe we could have a brief chat next week to talk through some potential solutions?

@aymanhab, I've changed hasPath() to hasVisualPath(), which will indicate if the Force object has a path that can be visualized. To support this, I've introduced AbstractPath::isVisualPath() which concrete implementations must provide. Hopefully this more directly addresses the needs of the GUI while not changing things too much.

aymanhab

Reviewed 5 of 62 files at r1, 24 of 43 files at r3, 9 of 16 files at r4, 2 of 9 files at r5, 4 of 8 files at r7, 10 of 18 files at r9, 11 of 11 files at r10, all commit messages.
Reviewable status: 69 of 70 files reviewed, 26 unresolved discussions (waiting on @adamkewley and @nickbianco)

OpenSim/Simulation/Model/Model.cpp line 326 at r10 (raw file):

            }
        }
        if (versionNumber < 40500) {

@nickbianco Typically when we change xml representation of a class, it becomes responsible for its own update so knowledge about the class changes are kept local and the Model class doesn't grow into a bag of unrelated changes that it shouldn't be concerned with. Can you comment on whether its possible/difficult to move this change to another more localized place? Thank you

nickbianco · 2023-09-01T22:01:30Z

@aymanhab I moved the XML updates to Force, it's much cleaner now.

aymanhab

Reviewed 1 of 4 files at r11.
Reviewable status: 68 of 71 files reviewed, 26 unresolved discussions (waiting on @adamkewley and @nickbianco)

aymanhab

Reviewable status: 68 of 71 files reviewed, 26 unresolved discussions (waiting on @adamkewley and @nickbianco)

OpenSim/Simulation/Model/Model.cpp line 326 at r10 (raw file):

Previously, aymanhab (Ayman Habib) wrote…

@nickbianco Typically when we change xml representation of a class, it becomes responsible for its own update so knowledge about the class changes are kept local and the Model class doesn't grow into a bag of unrelated changes that it shouldn't be concerned with. Can you comment on whether its possible/difficult to move this change to another more localized place? Thank you

Thank you 👍

adamkewley · 2023-09-06T17:06:09Z

@adamkewley, I've implemented your suggestions above for now. I'm thinking over your recent comments and taking a look ObjectProperty. This is getting into OpenSim territory (and, let's be honest, C++ concepts) I'm not as familiar with. Maybe we could have a brief chat next week to talk through some potential solutions?

@aymanhab, I've changed hasPath() to hasVisualPath(), which will indicate if the Force object has a path that can be visualized. To support this, I've introduced AbstractPath::isVisualPath() which concrete implementations must provide. Hopefully this more directly addresses the needs of the GUI while not changing things too much.

We can have a chat about it, but don't let that hold anything up!

nickbianco · 2023-09-06T21:18:42Z

@adamkewley replacing the new virtual method with AbstractProperty::assign() seems to work fine locally. If all the tests pass here, I'll go ahead and merge. Thanks again!

nickbianco added 5 commits August 23, 2023 16:21

First pass at implementing AbstractPath

34daa7a

Update examples and tests after GeometryPath API changes

3d5c2ee

Merge branch 'main' into wrapping_abstract_path

e0f19be

Safely initialize GeometryPath for path-based objects

81d1aee

Update changelog // use updPath instead of upd_path

b4f11fd

nickbianco requested review from aymanhab and adamkewley August 26, 2023 00:38

Update the Java bindings with path changes

b70b2fa

adamkewley reviewed Aug 29, 2023

View reviewed changes

nickbianco added 6 commits August 29, 2023 10:31

Revert Appearance to unnamed property

5e98a45

Add deprecated functions (e.g., getGeometryPath()) back in

4f81105

Reverting API changes pt. 2 + diff clean up

016d8bd

Reverting API changes pt. 3 + more diff clean up

bb4b8be

More diff clean up

5a74515

Stop IDE from removing trailing whitespaces...

ce3b0c4

adamkewley reviewed Aug 30, 2023

View reviewed changes

nickbianco added 2 commits August 30, 2023 10:04

Rule of five; more code clean up

c383f64

Handle unsupported path types when replacing muscles

e57c1f6

aymanhab requested a review from adamkewley August 30, 2023 17:21

aymanhab reviewed Aug 30, 2023

View reviewed changes

Remove initGeometryPath() for now

e53ea0b

aymanhab reviewed Aug 30, 2023

View reviewed changes

Backwards compatibility for deserializing GeometryPath

c3ac610

adamkewley reviewed Aug 31, 2023

View reviewed changes

nickbianco added 4 commits August 31, 2023 15:06

Add abstraction for path copying

df76b0a

Move path properties back to public scope

f64e3cd

More reverted code

f6e7208

Remove unnecessary includes

2940c77

nickbianco marked this pull request as ready for review August 31, 2023 22:41

aymanhab requested a review from adamkewley September 1, 2023 04:39

aymanhab requested changes Sep 1, 2023

View reviewed changes

adamkewley reviewed Sep 1, 2023

View reviewed changes

Rename copyFrom() to assign(); change hasPath() to hasVisualPath()

25ed001

aymanhab requested a review from adamkewley September 1, 2023 19:55

aymanhab requested changes Sep 1, 2023

View reviewed changes

nickbianco added 2 commits September 1, 2023 14:28

Move backwards compatibility for GeometryPath deserialization to Force

e9e0008

Minor whitespace cleanup

507e4e1

aymanhab approved these changes Sep 5, 2023

View reviewed changes

Use AbstractProperty::assign() instead of a new virtual method

83577ea

Flesh out changelog

83959bb

nickbianco merged commit 3f1fe38 into main Sep 7, 2023
6 checks passed

nickbianco deleted the wrapping_abstract_path branch September 7, 2023 18:42

tkuchida mentioned this pull request Mar 7, 2024

Add get_GeometryPath as a wrapper function for getGeometryPath in OpenSim 4.5 #3732

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement `AbstractPath` and related API changes to support multiple path types #3535

Implement `AbstractPath` and related API changes to support multiple path types #3535

nickbianco commented Aug 26, 2023 •

edited by aymanhab

Loading

nickbianco commented Aug 26, 2023

nickbianco commented Aug 28, 2023

adamkewley left a comment

adamkewley Aug 29, 2023

nickbianco Aug 29, 2023

adamkewley Aug 29, 2023

adamkewley left a comment

adamkewley Aug 30, 2023

aymanhab left a comment

aymanhab left a comment

adamkewley left a comment

adamkewley Aug 31, 2023

adamkewley Aug 31, 2023

nickbianco commented Aug 31, 2023

aymanhab left a comment

adamkewley left a comment

nickbianco commented Sep 1, 2023 •

edited

Loading

adamkewley commented Sep 1, 2023 •

edited

Loading

nickbianco commented Sep 1, 2023

aymanhab left a comment

nickbianco commented Sep 1, 2023

aymanhab left a comment

aymanhab left a comment

adamkewley commented Sep 6, 2023

nickbianco commented Sep 6, 2023

Implement AbstractPath and related API changes to support multiple path types #3535

Implement AbstractPath and related API changes to support multiple path types #3535

Conversation

nickbianco commented Aug 26, 2023 • edited by aymanhab Loading

Brief summary of changes

Testing I've completed

Looking for feedback on...

CHANGELOG.md (choose one)

nickbianco commented Aug 26, 2023

nickbianco commented Aug 28, 2023

adamkewley left a comment

Choose a reason for hiding this comment

adamkewley Aug 29, 2023

Choose a reason for hiding this comment

nickbianco Aug 29, 2023

Choose a reason for hiding this comment

adamkewley Aug 29, 2023

Choose a reason for hiding this comment

adamkewley left a comment

Choose a reason for hiding this comment

adamkewley Aug 30, 2023

Choose a reason for hiding this comment

aymanhab left a comment

Choose a reason for hiding this comment

aymanhab left a comment

Choose a reason for hiding this comment

adamkewley left a comment

Choose a reason for hiding this comment

adamkewley Aug 31, 2023

Choose a reason for hiding this comment

adamkewley Aug 31, 2023

Choose a reason for hiding this comment

nickbianco commented Aug 31, 2023

aymanhab left a comment

Choose a reason for hiding this comment

adamkewley left a comment

Choose a reason for hiding this comment

nickbianco commented Sep 1, 2023 • edited Loading

adamkewley commented Sep 1, 2023 • edited Loading

nickbianco commented Sep 1, 2023

aymanhab left a comment

Choose a reason for hiding this comment

nickbianco commented Sep 1, 2023

aymanhab left a comment

Choose a reason for hiding this comment

aymanhab left a comment

Choose a reason for hiding this comment

adamkewley commented Sep 6, 2023

nickbianco commented Sep 6, 2023

Implement `AbstractPath` and related API changes to support multiple path types #3535

Implement `AbstractPath` and related API changes to support multiple path types #3535

nickbianco commented Aug 26, 2023 •

edited by aymanhab

Loading

nickbianco commented Sep 1, 2023 •

edited

Loading

adamkewley commented Sep 1, 2023 •

edited

Loading