Modify

Opened 7 years ago

Last modified 6 years ago

#4786 new Feature Requests

custom property_tree

Reported by: anonymous Owned by: Sebastian Redl
Milestone: To Be Determined Component: property_tree
Version: Boost 1.44.0 Severity: Problem
Keywords: property_tree Cc: saleyn@…

Description

When creating custom property_trees by using a data type different from std::string, property_tree has errors. Attached is a patch along with an implementation of a tree containing variant data type.

Attachments (5)

boost_1_44_0.property_tree.patch (3.2 KB) - added by saleyn@… 7 years ago.
variant_property_tree.tgz (5.3 KB) - added by saleyn@… 7 years ago.
Variant property tree
boost_1_44_0.property_tree.2.patch (3.3 KB) - added by saleyn@… 7 years ago.
Please use this patch instead, the previous upload broke some legacy functionality.
variant_property_tree-1_49_0.tgz (40.0 KB) - added by saleyn 6 years ago.
Sample of a variant property tree with typed node data (boost version 1_49_0
variant_tree_boost_1_49_0.tgz (7.4 KB) - added by saleyn 6 years ago.
Version working without needing to patch boost

Download all attachments as: .zip

Change History (20)

Changed 7 years ago by saleyn@…

Changed 7 years ago by saleyn@…

Attachment: variant_property_tree.tgz added

Variant property tree

Changed 7 years ago by saleyn@…

Please use this patch instead, the previous upload broke some legacy functionality.

comment:1 Changed 7 years ago by viboes

Please, could you add the kind or errors you get?

comment:2 Changed 7 years ago by Serge Aleynikov <saleyn@…>

You can just untar the variant_property_tree.tgz and try to compile the project with latest version of property_tree to get the errors. If that's not enough, I can post the actual errors.

comment:3 Changed 6 years ago by anonymous

the idea looks nice. the C-style casts don't. the try/catch cascade with lexical_cast for guessing the type from string looks even more horrible, but there's nothing else to do since INFO files don't have any hints like "this property has that type" inside. it's too bad that the library maintainer didn't pay attention yet, maybe he could suggest something.

comment:4 Changed 6 years ago by Sebastian Redl

Type: BugsFeature Requests

The INFO parser (like all other parsers) only supports string trees. That's just the way it is. I may enhance the parsers in the future. However, wildly casting pointers is *not* the way to do it.

Marking this as a feature request.

comment:5 in reply to:  4 Changed 6 years ago by saleyn

Replying to cornedbee:

The INFO parser (like all other parsers) only supports string trees. That's just the way it is. I may enhance the parsers in the future. However, wildly casting pointers is *not* the way to do it.

Marking this as a feature request.

I agree that the casts were somewhat ugly, but supporting the feature of having derived trees with typed data nodes favored by the property_tree's underlying parsing architecture would be a great add-on. I couldn't see the way of doing it cleanly in the current implementation without needing to patch it at least minimally. So if you can introduce this feature given the derived version of the variant_tree that I submitted above it would be great!

comment:6 in reply to:  3 Changed 6 years ago by saleyn

Replying to anonymous:

the idea looks nice. the C-style casts don't. the try/catch cascade with lexical_cast for guessing the type from string looks even more horrible, but there's nothing else to do since INFO files don't have any hints like "this property has that type" inside. it's too bad that the library maintainer didn't pay attention yet, maybe he could suggest something.

Ideally the parsers would minimally give an ability to infer the type based on the parsed data, and provide a hint to the derived implementation so that the commonly identifiable inputs "1", "2.0", '"string"' are transformed into statically overridable calls that the transform function would be able to map into properly typed objects, e.g.:

enum hint_t {UNDEFINED, INT, FLOAT, STRING};

std::pair<std::basic_string<Ch>, hint_t> raw_data = read_data(text, &need_more_lines); typename Ptree::data_type typed_data = transform_data(raw_data); last.data() = typed_data;

comment:7 Changed 6 years ago by anonymous

that last idea of guessing the type by its looks sounds dangerous to me. it may prove even worse than the try/catch/lexical_cast cascade. it's better to provide a way to supply some "schema" to the parser so he would know each property's type. explicit is better than implicit, as they say.

comment:8 Changed 6 years ago by Sebastian Redl

I can see the point in making the JSON parser variant-tree-aware. JSON is already typed, after all, and the current parser throws away information.

But all this talk about how to shoehorn INFO into a typed tree doesn't make sense to me. INFO is not any official format. It was invented by the original PTree author specifically for string ptrees. If you want types, then why not just adjust the format to allow type annotations?

comment:9 in reply to:  7 Changed 6 years ago by saleyn

Replying to anonymous:

that last idea of guessing the type by its looks sounds dangerous to me. it may prove even worse than the try/catch/lexical_cast cascade. it's better to provide a way to supply some "schema" to the parser so he would know each property's type. explicit is better than implicit, as they say.

Agree. The explicit schema would be a much better solution. Especially if that schema can be uniformly adapted across all ptree's parsers.

comment:10 in reply to:  8 Changed 6 years ago by saleyn

Replying to cornedbee:

I can see the point in making the JSON parser variant-tree-aware. JSON is already typed, after all, and the current parser throws away information.

But all this talk about how to shoehorn INFO into a typed tree doesn't make sense to me. INFO is not any official format. It was invented by the original PTree author specifically for string ptrees. If you want types, then why not just adjust the format to allow type annotations?

IMHO, INFO is a very succinct and convenient format. It's conceptually very similar to Erlang term encoding format, which is quite popular. In fact most of dynamically typed languages use standard implicit rules to parse 1234 as integers, 234.5 or 2.345e2 as floats, "1234" as strings, and true or false as booleans (just like JSON protocol does, except that it doesn't distinguish integers from floats and treats them as "number"). So, adapting that idea to the INFO parser so that it can use a custom translator to allow a user to take advantage of that extra metadata wouldn't be that bad.

comment:11 Changed 6 years ago by saleyn

For what it's worth, I converted the variant tree example to the latest boost version 1.49.0, and it turned out that there's a way to get it working without a need for patching the source. The latest example is uploaded as attachment.

Following this thread I do like the schema idea, but this is definitely not a bug request.

Changed 6 years ago by saleyn

Sample of a variant property tree with typed node data (boost version 1_49_0

comment:12 Changed 6 years ago by anonymous

Some way to specify a custom default translator in basic_ptree would be really useful. I'm talking about the last example. That inheritance from basic_ptree with all those handcrafted member functions looks cumbersome (and also somewhat dangerous - there's no virtual destructor in basic_tree).

comment:13 Changed 6 years ago by anonymous

a few more notes regarding your example. util::detail::variant_translator::get_value seems to lack support for converting the internal value to std::string (except for the case when its type is also std::string). and if you pass std::string to util::detail::variant_translator::put_value, it looks like it will just replace the internal value with the supplied string, changing its type to std::string, not converting the string to the current internal type. maybe it's better to add util::detail::variant_translator<std::string> specialization to improve this. correct me if I've missed anything. :)

Changed 6 years ago by saleyn

Version working without needing to patch boost

comment:14 Changed 6 years ago by saleyn

Upon checking my last submission, I noticed that I uploaded a stale version which still didn't work without patching the tree. variant_tree_boost_1_49_0.tgz solves that problem. The work-around was to apply a "post-conversion" transform after reading data from the info file to convert strings to corresponding variant types.

Concerning the last comment, the variant_translator has the following code that makes sure that std::string is one of the types it handles (variant::valid_types type includes std::string):

        template<typename T>
        typename boost::disable_if<
            boost::is_same<
                boost::mpl::end<valid_types>::type,
                boost::mpl::find<variant::valid_types, T>
            >,
            internal_type>::type
        put_value(T value) const {
            return variant(value);
        }

Since variant_tree::put_value(value) uses default translator, it should be able to handle std::string conversion. If you don't believe it does, please include a test case in the test_variant.cpp that illustrates a failing case.

comment:15 Changed 6 years ago by anonymous

hmm, well, I see a two-argument variant constructor that calls from_string (this seems to be the only place where the conversion from string takes place), but I can't figure out where is that constructor used :)

Modify Ticket

Change Properties
Set your email in Preferences
Action
as new The owner will remain Sebastian Redl.

Add Comment


E-mail address and name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.