Initializer list and insert/update chaining #1359
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
varunagrawal
approved these changes
Dec 31, 2022
| /* ************************************************************************ */ | ||
| bool DiscreteValues::equals(const DiscreteValues& x, double tol) const { | ||
| if (this->size() != x.size()) return false; | ||
| for (const auto values : boost::combine(*this, x)) { |
There was a problem hiding this comment.
Seems odd to use boost here if you want to eventually remove it.
| /// @{ | ||
|
|
||
| // insert in base class; | ||
| std::pair<iterator, bool> insert( const value_type& value ){ |
There was a problem hiding this comment.
Can't we do using Base::insert here?
| * stores cardinality of a Discrete variable. It should be handled naturally in | ||
| * the new class DiscreteValue, as the variable's type (domain) | ||
| /** | ||
| * A map from keys to values |
There was a problem hiding this comment.
A map from gtsam::Keys to a discrete value each.
| /// @name Testable | ||
| /// @{ | ||
|
|
||
| /// print required by Testable. |
There was a problem hiding this comment.
/// GTSAM-style print method
| void print(const std::string& s = "", | ||
| const KeyFormatter& keyFormatter = DefaultKeyFormatter) const; | ||
|
|
||
| /// equals required by Testable for unit testing. |
There was a problem hiding this comment.
/// GTSAM-style test method
| /** For all key/value pairs in \c values, replace values with corresponding | ||
| * keys in this object with those in \c values. Throws std::out_of_range if | ||
| * any keys in \c values are not present in this object. */ | ||
| DiscreteValues& update(const DiscreteValues& values); |
There was a problem hiding this comment.
I'll add insert_or_assign to this later since I need to do it for HybridValues too.
|
|
||
| static const VectorValues kExample = {{99, Vector2(2, 3)}}; | ||
|
|
||
| // Check insert |
There was a problem hiding this comment.
Pedantic but missing test demarcation.
|
Ok, will merge if you will take care of comments in next PR |
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.
In anticipation of removing boost altogether, I made sure initializer lists worked for VectorValues, DiscreteValues, and HybridValues. Also modernized insert/update.