Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

use @defVariable for RoME Variable Types #328

Merged
merged 7 commits into from
Oct 27, 2020
Merged

Conversation

Affie
Copy link
Member

@Affie Affie commented Aug 25, 2020

Just testing it for now. Needs DFG v0.10.2

@Affie Affie added this to the v0.x.0 milestone Aug 25, 2020
@dehann
Copy link
Member

dehann commented Aug 25, 2020

towards #244

@dehann dehann modified the milestones: v0.x.0, v0.10.0 Sep 1, 2020
@dehann
Copy link
Member

dehann commented Sep 1, 2020

shall we target a minor release on RoME then, v0.10.0 is next. This is really great! Thanks for doing!


#uses DFG v0.10.2 @defVariable for above
include("variables/VariableTypes.jl")
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@dehann is it ok to have all variable definitions in one file like this?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah, saw you did that -- think that will work well thanks otherwise they are all over the place -- good call!

@Affie
Copy link
Member Author

Affie commented Sep 1, 2020

Still to do is to update all the test that uses the old softtype ut fields.

@Affie
Copy link
Member Author

Affie commented Sep 3, 2020

Maybe convert on nstime from second, millisecond, float, etc.

@codecov-commenter
Copy link

codecov-commenter commented Sep 3, 2020

Codecov Report

Merging #328 into master will increase coverage by 1.48%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #328      +/-   ##
==========================================
+ Coverage   15.19%   16.68%   +1.48%     
==========================================
  Files          43       43              
  Lines        2145     2200      +55     
==========================================
+ Hits          326      367      +41     
- Misses       1819     1833      +14     
Impacted Files Coverage Δ
src/RoME.jl 100.00% <ø> (ø)
src/variables/deprecated/DynPoint2D.jl 0.00% <ø> (ø)
src/variables/deprecated/DynPose2D.jl 0.00% <ø> (ø)
src/variables/deprecated/Point2D.jl 0.00% <ø> (ø)
src/variables/deprecated/Point3D.jl 0.00% <ø> (ø)
src/variables/deprecated/Pose2D.jl 0.00% <ø> (ø)
src/variables/deprecated/Pose3D.jl 0.00% <ø> (ø)
... and 24 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e5520c4...829d5b3. Read the comment docs.

@dehann
Copy link
Member

dehann commented Oct 21, 2020

this will cross paths with #341 sooner or later.

@dehann dehann marked this pull request as ready for review October 27, 2020 18:09
@dehann dehann merged commit 6a9e57f into master Oct 27, 2020
@GearsAD
Copy link
Collaborator

GearsAD commented Oct 27, 2020

Awesome, thanks!

@Affie Affie deleted the maint/20Q3/use@defVariable branch February 1, 2021 07:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants