Clean up constraint namespace #342
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add shorter aliases in aikido/constraint.hpp.
Codecov Report
@@ Coverage Diff @@
## master #342 +/- ##
==========================================
+ Coverage 81% 81.02% +0.01%
==========================================
Files 205 205
Lines 6003 6003
==========================================
+ Hits 4863 4864 +1
+ Misses 1140 1139 -1
|
jslee02
reviewed
Feb 21, 2018
| using dart::createDifferentiableBounds; | ||
| using dart::createProjectableBounds; | ||
| using dart::createTestableBounds; | ||
| using dart::createSampleableBounds; |
There was a problem hiding this comment.
It seems using namespace dart (in aikido::constraint namespace) also works here instead of listing all the classes. But I'm not sure if this is a good practice. @psigen Could you shed some light on here? 😄
jslee02
approved these changes
Feb 23, 2018
| @@ -51,7 +51,7 @@ class InteractiveMarkerViewer | |||
| /// \param basename Basename for markers | |||
| /// \return TSRMarkerPtr contains sampled frames of TSR. | |||
| TSRMarkerPtr addTSRMarker( | |||
| const aikido::constraint::TSR& tsr, | |||
| const aikido::constraint::dart::TSR& tsr, | |||
There was a problem hiding this comment.
Nit: Let's simplify this to const constraint::dart::TSR.
| #include "aikido/constraint/uniform/RnConstantSampler.hpp" | ||
|
|
||
| #undef dtwarn | ||
| #define dtwarn (::dart::common::colorErr("Warning", __FILE__, __LINE__, 33)) |
There was a problem hiding this comment.
Is this because of namespace dart ambiguous between ::dart and ::aikido::[~]::dart? Let me make an upstream fix for this.
There was a problem hiding this comment.
Yep. I created #348 to track this.
src/rviz/InteractiveMarkerViewer.cpp
Outdated
| int nSamples, | ||
| const std::string& basename) | ||
| { | ||
| using dart::dynamics::Frame; | ||
| using dart::dynamics::SimpleFrame; | ||
| using aikido::constraint::TSR; | ||
| using aikido::constraint::dart::TSR; |
There was a problem hiding this comment.
Nit: Let's remove leading namespace aikido::.
src/rviz/InteractiveMarkerViewer.cpp
Outdated
| @@ -61,13 +61,13 @@ FrameMarkerPtr InteractiveMarkerViewer::addFrame( | |||
|
|
|||
| //============================================================================== | |||
| TSRMarkerPtr InteractiveMarkerViewer::addTSRMarker( | |||
| const aikido::constraint::TSR& tsr, | |||
| const aikido::constraint::dart::TSR& tsr, | |||
There was a problem hiding this comment.
Nit: Let's remove leading namespace aikido::.
| @@ -231,7 +231,7 @@ std::unique_ptr<aikido::trajectory::Spline> planToEndEffectorPose( | |||
| = std::make_shared<aikido::constraint::TestableIntersection>(stateSpace); | |||
| compoundConstraint->addConstraint(constraint); | |||
| compoundConstraint->addConstraint( | |||
| aikido::constraint::createTestableBounds(stateSpace)); | |||
| aikido::constraint::dart::createTestableBounds(stateSpace)); | |||
There was a problem hiding this comment.
Nit: Let's remove leading namespace aikido::.
| @@ -181,7 +181,7 @@ std::unique_ptr<aikido::trajectory::Spline> planToEndEffectorOffset( | |||
| = std::make_shared<aikido::constraint::TestableIntersection>(stateSpace); | |||
| compoundConstraint->addConstraint(constraint); | |||
| compoundConstraint->addConstraint( | |||
| aikido::constraint::createTestableBounds(stateSpace)); | |||
| aikido::constraint::dart::createTestableBounds(stateSpace)); | |||
There was a problem hiding this comment.
Nit: Let's remove leading namespace aikido::.
| @@ -181,7 +181,7 @@ std::unique_ptr<aikido::trajectory::Spline> planToEndEffectorOffset( | |||
| = std::make_shared<aikido::constraint::TestableIntersection>(stateSpace); | |||
There was a problem hiding this comment.
Nit: Let's remove leading namespace aikido::.
gilwoolee
pushed a commit
that referenced
this pull request
Jan 21, 2019
* Create constraint::uniform namespace. Add shorter aliases in aikido/constraint.hpp. * Create constraint::dart namespace. * Fix tests after introducing constraint::dart. * Update CHANGELOG.md. * Address @jslee02's comments.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
constraint::uniformnamespace to mirror the existing directory structureconstraint::dartnamespaceBefore creating a pull request
make formatBefore merging a pull request
CHANGELOG.md