Un-deprecate NoiseModelFactor1-6
and move X1
and key1-6
shortcuts to NoiseModelFactorN
#1370
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.
Following the discussion from #1350, this PR replaces #1350 and undoes some changes from #1344:
X1
-X6
aliases intoNoiseModelFactorN
(not deprecated)key1()
-key6()
shortcuts inNoiseModelFactorN
NoiseModelFactor1
in withNoiseModelFactorN
in pre-made factors #1344 due to templates on dependent types. The code should still work fine without undo-ing, but I think it's easier to read this way.#define NoiseModelFactor1 NoiseModelFactorN
. Now thatNoiseModelFactorN
's API is a superset ofNoiseModelFactor1-6
, we can simply define it so.As for 3,
template <T> using NoiseModelFactor1 = NoiseModelFactorN<T>
doesn't work because some detail regarding templates and class name injection. See #947 (comment)Primary change is in
NonlinearFactor.h
. The rest of the files are just undo-ing some changes from #1344 and un-disabling deprecation warnings intestNonlinearFactor.h
.Copy-pasting pros and cons from previous discussion for reference:
Pros:
NoiseModelFactor1-6
toNoiseModelFactorN
way easier, since no calling-side code has to change at all. Just find-all-replace: public NoiseModelFactor[1-6]
->: public NoiseModelFactorN
MyFactor::X3
is (subjectively) cleaner thanMyFactor::ValueType<3>
.X7
, then they can just useValueType<7>
.ValueType<1>
annoying to use: when you are defining a templated factor and you want to useValueType<1>
inside the definition of the templated factor, you need to dotypename MyFactor::template ValueType<1>
instead of justValueType<1>
. So in these cases, it's much nicer to just useX1
.Cons:
NoiseModelFactorN
making it slightly difficult to read (now inherits fromdetail::NoiseModleFactorAliases<ValueTypes>
).#define NoiseModelFactor1 NoiseModelFactorN
, some compilers may not support deprecating the old classes, and generally#define
is kind of bad practice (?)