-
Notifications
You must be signed in to change notification settings - Fork 7k
[ntuples] Add header-only C++ ntuples library 0.1.2 added port of NTUPLES library #46932
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
base: master
Are you sure you want to change the base?
Conversation
@microsoft-github-policy-service agree |
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.
I pushed fixes for the comments.
ports/ntuples/portfile.cmake
Outdated
add_library(ntuples INTERFACE) | ||
add_library(ntuples::ntuples ALIAS ntuples) | ||
target_include_directories(ntuples INTERFACE \"\${_IMPORT_PREFIX}/include\") | ||
target_compile_features(ntuples INTERFACE cxx_std_20) |
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.
target_compile_features(ntuples INTERFACE cxx_std_20) | |
target_compile_features(ntuples INTERFACE cxx_std_17) |
https://github.com/RPeschke/ntuples/blob/e3b124f148678fa3cfa2780579948a62df7da23e/CMakeLists.txt#L28
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.
Hi,
I am sorry but it will not compile with c++17
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.
I am sorry but it will not compile with c++17
I see, in that case upstream should be fixed so that we aren't randomly doing something different than upstream. (Doesn't mean upstream needs to change before we would merge this, just that we would want upstream's approval before doing something different)
I made the original comment and pushed the change without realizing that you were also the upstream maintainer 😅. You should probably fix that and remove the attempt to set CMAKE_CXX_STANDARD
.
I got so distracted by header-only that I forgot to check the name :( repology doesn't suggest this name is already used for that repo and trying to google the name in the context of C or C++ seems to point everywhere to GSL ( https://www.gnu.org/software/gsl/doc/html/ntuple.html ) or to Would you accept changing the name of the port to |
Tagging vcpkg-team-review for the name. On balance I think the name is probably fine because I think there's little chance of actual confusion but |
OK thanks. Let me see how how i can change the name. Does the Entire library need a new name or only the port (the name inside vcpkg) |
Just the name inside vcpkg. I can push that change for you if you're OK with it. |
maybe we can called it something like named_tuple that would be short and descriptive however we can also go for rp-ntuples |
I still need to ask for approval from another maintainer for those. Decision tree is something like:
Thanks for the new port; will keep you posted. |
the library requires cxx_std_20 to compile
Hi, I don't understand the 2 failing checks. Can you help me with that? Thanks, |
The version database needs fixed because the port has changed. I would normally just push a fix for that but I haven't bothered while I'm waiting for another name approval. |
ports/ntuples/vcpkg.json
Outdated
"vcpkg-cmake", | ||
"vcpkg-cmake-config" |
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.
These should be host dependencies.
ports/ntuples/portfile.cmake
Outdated
vcpkg_install_copyright(FILE_LIST | ||
"${SOURCE_PATH}/LICENSE" "${SOURCE_PATH}/LICENSE.txt" |
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.
vcpkg_install_copyright(FILE_LIST | |
"${SOURCE_PATH}/LICENSE" "${SOURCE_PATH}/LICENSE.txt" | |
vcpkg_install_copyright(FILE_LIST | |
"${SOURCE_PATH}/LICENSE" |
We don't need 2 copies of the same file :)
@ras0219-msft @vicroms and I discussed today. We can't merge We can merge this without changing the "real library name" or where the headers are installed, but adding a prefix or some other distinguishing name is likely to help you and your users because it will make you easy to Google. (It seems unlikely that the search conflicts with GSL ntuples and/or std::tuple would be resolved soon :) ) |
OK I think rp-ntuples is fine. Thanks for your help |
Summary
Add a new port ntuples: a header-only C++20 library for named-field tuples and lightweight dataframe-style helpers.
What this port does
core/include/
toinclude/
.share/ntuples/ntuples-config.cmake
that defines the targetntuples::ntuples
.target_compile_features(… cxx_std_20)
.share/ntuples/usage
file so users see clear usage instructions after install.CMake usage
Example Use
This is just a header-only C++20 library. It uses ordinary C++ (templates, operator overloading, a few macros). No new language, no code generation, no external reflection.
Named fields at compile time.
nt_new_field_name(index); nt_new_field_name(index_squared);
declares tag types used as field names.Create records on the fly.
nt::ntuple(index = i, index_squared = i * i, nt_field(cubed) = i * i * i)
constructs a lightweight record.Known fields use the tags above;
nt_field(cubed)
shows an ad-hoc field defined inline.Composes with range pipelines.
Ideal for using with range library.
Ergonomic access.
Access fields by name (
t.cubed
) or by index (nt::get<0>(t)
), with compile-time type checking.