-
Notifications
You must be signed in to change notification settings - Fork 789
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
Feature/vector value print #68
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Mandy, thanks !!! But I don't see how a simple print change should change so many things? Are you sure you have the latest develop? And can you restrict the PR to just the print?
Finally, I suggest you make a unit test. To make that work, make a stream operator <<
that you call from Print, and do the test with stringstream...
Sorry, I will update my pr to the latest develop. However, I have some questions. |
If you look at the "changed files" you see that there are differences all over the file. Did you by chance re-format? I'd prefer if that is not done in GTSAM, to make sure the PR and commits are limited to the new functionality. You are right about the KeyFormatter - that is an issue that we will have to confront when we switch to streams. I think there is a solution with stream manipulators, let me investigate a bit... |
9fe28e4
to
9dc6b9d
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just a check, will fix the extra changes.
9dc6b9d
to
7b41007
Compare
Could you use the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Getting there !
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Two small comments remain
OK, cool, PR #69 is merged which you can use to send the formatter into the stream:
|
5ddaff8ba Merge pull request #77 from borglab/fix/template-as-template-arg 0b6f2d92b allow templates as paramters to templates 7f3e242b0 Merge pull request #76 from borglab/fix/cmake-config 0caa79b82 macro to find and configure matlab 522557232 fix GTWRAP_INCLUDE_NAME 78a5d3afa Use CMakePackageConfigHelpers to vastly simplify the package config 76f8b9e5d Merge pull request #75 from borglab/fix/template-args 3b8e8389e remove reference from shared pointers 045393c7b docs and flag renaming d23d8beae tests ef96b4bdc don't make template parameters as references d1e1dc697 Merge pull request #74 from borglab/fix/type-recursion 8202ecf10 minor fixes 5855ea85b support for passing templated types as arguments 150cc0578 Support for templated return types 5c58f8d03 Merge pull request #73 from borglab/fix/types-refactor c697aa9c8 refactored the basic and custom types to make it cleaner, added more tests 98e2c3fa1 Merge pull request #68 from borglab/fix/cmake c6d5e786a make config agnostic to install prefix 4d6999f15 Merge pull request #69 from borglab/feature/call-and-index ccf408804 add support for callable and indexing overloads 8f8e3ec93 add status messages 88566eca4 make WRAP_PYTHON_VERSION an optional argument 01b8368ad Merge pull request #67 from borglab/feature/operator-overloading 522a12801 remove unsupported operators 209346133 update check location for unary operator 39e290f60 fix small typo faa589bec update DOCS 7ff83cec8 minor fixes 8ce37766f fixed tests 21c477c4d include pybind11/operators a3534ac5e wrap operator overloads 67c8f2089 instantiate templates for operators e9dce65d8 use ReturnType for ease in other places and use members in Class 3078aa6db added parser rule for operator overloading git-subtree-dir: wrap git-subtree-split: 5ddaff8bab6c05e8a943c94993bf496e13296dd6
This change is