Skip to content
New issue

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

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

Already on GitHub? # to your account

Support conversion between vector<Any> and vector<typename T::value_type> #873

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

dyackzan
Copy link

@dyackzan dyackzan commented Sep 24, 2024

These changes make way for us to do general vector operations like iterate over, index, append, and remove using BT.CPP. Using vector<Any> objects on the blackboard allows us to create these operations as singletons, rather than requiring each operation to be templated for each vector<T>

The 2 primary components to this PR:

  • In setOutput, convert vector<T> objects to vector<Any> before placing them on the blackboard
  • In getInput, attempt to unwrap and convert vector<Any> to vector<T> if a vector<T> is expected

Don't check port type alignment for vector<Any>
@dyackzan dyackzan marked this pull request as ready for review September 24, 2024 17:44
@dyackzan dyackzan force-pushed the support-vec-any-to-vec-type-conversion branch from 3e23456 to d55f69e Compare October 3, 2024 17:33
@dyackzan dyackzan changed the title Support vector<Any> -> vector<typename T::value_type> conversion Support conversion between vector<Any> and vector<typename T::value_type> Oct 3, 2024
@dyackzan dyackzan force-pushed the support-vec-any-to-vec-type-conversion branch from d55f69e to fbb6583 Compare October 7, 2024 15:40
@dyackzan dyackzan force-pushed the support-vec-any-to-vec-type-conversion branch 5 times, most recently from 29c90be to 7d0219e Compare October 23, 2024 22:47
Also update checks to allow mismatch when a port was declared as a
vector<T> and we have an input port that takes it in as a vector<Any>
@dyackzan dyackzan force-pushed the support-vec-any-to-vec-type-conversion branch from 7d0219e to f8493e5 Compare October 23, 2024 22:48
Copy link

@nbbrooks nbbrooks left a comment

Choose a reason for hiding this comment

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

Could you document the gcc and clang demangled output for the std::vector<std::string> for reference here in the PR if it isn't possible to add a unit test establishing the demangled output passes the regex max? Ideally we'd have a unit test for partial specialization as well.

Co-authored-by: Nathan Brooks <nbbrooks@gmail.com>
@dyackzan
Copy link
Author

I checked clang and gcc with the type demangler and get the the same output for the demangled typeid of a std::vector<string> for both:

std::vector<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::allocator<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > > >

@nbbrooks
Copy link

Oops, when I hit approve I lost track of my tabs and thought I was approving a submodule update to point to this PR branch 🤦

@facontidavide
Copy link
Collaborator

looks like a good addition. Can you please:

  • use pre-commit to fix the formatting?
  • add a simple unit test too?

@dyackzan dyackzan force-pushed the support-vec-any-to-vec-type-conversion branch from 55a9f5a to 6607968 Compare November 6, 2024 15:39
@dyackzan
Copy link
Author

dyackzan commented Nov 6, 2024

How does that look @facontidavide ?

@dyackzan
Copy link
Author

dyackzan commented Dec 2, 2024

Tagging @facontidavide , could you give me another review on this?

@dyackzan
Copy link
Author

Friendly ping @facontidavide , please let me know if there's anything else you'd like me to change here. We want to keep our fork as up to date with your base as possible

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants