-
Notifications
You must be signed in to change notification settings - Fork 99
Add function to create gko::config::pnode from std::string #1721
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
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.
LGTM, I thought we had this already
/** | ||
* parse_json_string takes a json string to generate the property tree object | ||
*/ | ||
inline gko::config::pnode parse_json_string(std::string json) |
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 wonder if it's a good time to start introducing std::string_view
to the code base, now that we support 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.
need to add a test for it in extensions/test/json_config
question for pyGinkgo:
do you have any intermediate layer in C or C++ between ginkgo and pyGinkgo?
sure i'll add one
we use pybind11 , which is c++, to generate any intermediate code to register to python. |
09023e0
to
62d4f08
Compare
This PR adds a function to create
gko::config::pnode
directly from astd::string
.Motivation: We would like to use Ginkgo's file config solver within the pyGinkgo bindings. The current implementation of the config solver seems to rely on json files. However, creating temporary files is a nuisance which can cause all kinds issues and is not really necessary in our case. Thus, just passing a
std::string
to generate a property tree would simplify things a lot.