NEW FEATURE: Numeral with % should be interpreted as a float

[GoogleCodeExporter]

What steps will reproduce the problem?
1. In YAML: 'bar: "50%"'
2. class Foo { double bar; }
3. Foo x = (Foo) new Yaml(new Loader(new Costructor(Foo.class))).load(YAML)
4. Cannot convert from string to double.

What is the expected output? What do you see instead?
- Expected double bar == 0.5. Instead, recieve error.

What version of the product are you using? On what operating system?
- Snakeyaml trunk, 64b Ubuntu 10.04, 32b jvm : java-6-sun, mvn 2.2.1.

Please provide any additional information below.
- Attached is a patch that adds an optional percent-sign to the float-type 
regex, then handles removing the percent-sign and dividing by 100 (if present) 
when creating BigDecimal, Double, and Float objects.

Original issue reported on code.google.com by jon.p.he...@gmail.com on 28 Jul 2010 at 8:34

Attachments:

• percentToFloat.patch

[GoogleCodeExporter]

Sorry, futzed the ticket type. :<

The changes should be easy to read:
- FLOAT regex has two characters appended, namely "%?".
- Constructor checks to see if type is BigDecimal and does logic there. This 
logic is duplicated in the doubleConstructor in SafeConstructor for 
Double/Float.

Original comment by jon.p.he...@gmail.com on 28 Jul 2010 at 9:49

[GoogleCodeExporter]

[deleted comment]

[GoogleCodeExporter]

I did not quite catch you. Can you please provide more information ?

1) Since you use double quotes you explicitly mark a string. It should be 'bar: 
50%'

2) The YAML specification clearly defines the format for floats. How other YAML 
parsers shall work if they see 50% in a YAML document ?

3) The patch does not compile:
src\main\java\org\yaml\snakeyaml\constructor\Constructor.java:[443,43] operator 
/ cannot be applied to java.math.BigDecimal,int

4) The patch is only about constructor. How is the dumper affected by this 
change ?

5) there is no single test provided

I would prefer you to try to follow TDD:
- write an exhaustive set of tests. The failures will show where the code 
should be improved
- change the code to fix the tests

Thank you for your contribution.

Original comment by py4fun@gmail.com on 29 Jul 2010 at 9:35

• Added labels: Type-Enhancement
• Removed labels: Type-Defect

[GoogleCodeExporter]

1) It works fine as 'bar: 50%' in the repro above.
2) This is a optional enhancement for this parser, and only when the java 
object type is given explicitly. When the destination is a Java 
double/float/etc it parses '50%' into a double.  When the destination is a 
String it produces a String.  Prior to this patch if '50%' was destined for a 
double, snakeyaml would explode.  This patch causes snakeyaml to Do The Right 
Thing (tm).

  The output from snakeyaml hasn't been changed and will still produce code in the cannocial form defined by the YAML spec.  Thus, snakeyaml will never produce anything not readable by another compliant parser.  Likewise since snakeyaml still accepts all floats defined in the YAML spec, no other compliant parser will produce anything that snakeyaml can't parse.
3) I apologize for a clerical error on my part. This is the result of an 
accidental stale patch. I had packaged the jar (compiled everything, ran all 
tests) but neglected to update my patch before submission. Attached is the 
patch that should have been submitted.
4) The dumper is unaffected. Dumping a double to canonical form is well-defined 
and I haven't changed that.
5) See #3.

Original comment by jon.p.he...@gmail.com on 29 Jul 2010 at 6:16

Attachments:

• snakeyaml-percentToFloat.txt

[GoogleCodeExporter]

a) if the java object type is given explicitly then '50' shall create double 
without redundant '%'. If not it is a bug.
b) The test you provided does not cover lines 441-444 you added to 
Constructor.java. Run 'mvn clean site' and check the Cobertura report.
c) I see now my problem with the documentation. I am afraid what you need is to 
add am implicit resolver. Search for text 
(http://code.google.com/p/snakeyaml/wiki/Documentation):
"There is a way to teach SankeYAML that any untagged plain scalar that looks 
like XdY has the implicit tag !dice. Use Yaml.addImplicitResolver(String tag, 
Pattern regexp, String first) then you don't have to specify the tag to define 
a Dice object".
If this is really the case may I kindly ask you to help me to describe your 
situation in the documentation in a way anybody can spot it ?

Original comment by py4fun@gmail.com on 30 Jul 2010 at 8:54

[GoogleCodeExporter]

You should have included a reference to the cause of the issue:
https://issues.apache.org/jira/browse/CASSANDRA-1313

It gives the issue the context. 

When I read this "Whenever it sees a number followed by a percent, it should 
correctly construct a FloatType" it makes me think that custom implicit 
resolver is the way to go.

Original comment by py4fun@gmail.com on 30 Jul 2010 at 3:03

[GoogleCodeExporter]

An attached patch adds two tests to the previous patch.

The first covers lines 441-444 (BigDecimal parsing). Note that BigDecimal comes 
with IEEE floating point inaccuracy, so the assertion just proves that the 
conversion is within error.

The second test is to prove that the code, as it was first committed, only 
works when parsing into a given type explicitly. I seem to have confused you 
with the above sentence, so I'll reiterate, "[t]he output from snakeyaml hasn't 
been changed and will still produce code in the cannocial form defined by the 
YAML spec." 
Parsing "bar: 50%" with no other information produces the string "50%", as 
expected.

I'm convinced that a percentage is common usage, not custom usage. Furthermore, 
to make a custom DoubleConstructor that includes percents (and still has all 
the same functionality as a regular double constructor), you would have to 
override ConstructYamlFloat and effectively reimplement the whole thing.

Original comment by jon.p.he...@gmail.com on 2 Aug 2010 at 10:45

Attachments:

• snakeyaml-percentToFloat.txt

[GoogleCodeExporter]

I still do not understand why project specific logic should be implemented in 
general place:
- non-standard character %
- unexpected division by 100
- commas must not be removed (it makes it unclear whether the list [1,25%] has 
one or two values)
- comment: "We're looking at a time. Return a Double of seconds_since_00:00:00."
This is no time but number with the integer part in base 60

Imagine the case when a BASIC programmer asks to treat 123$ in a special way. 
Because this is a common usage (in BASIC they use it all over the place) let us 
treat all scalars which end with '$' as Strings. So 123$ should be '123'. I am 
afraid the rest of the community (who is not a BASIC programmer) will be only 
confused.

>I'm convinced that a percentage is common usage, not custom usage
- if it is common why it is only used when the runtime class is known ?
- for me when people talk about percentage they always give an approximate 
number (which is integer). So 50% is a norm, but 50.12345% is not normal. That 
is why it is unclear why you need BigDecimal for percentage 

There is a simple solution. Check this:  
http://code.google.com/p/snakeyaml/source/browse/src/test/java/examples/CustomBe
anResolverTest.java and 
http://code.google.com/p/snakeyaml/source/browse/src/test/java/examples/CustomIm
plicitResolverTest.java to see the solution without any need to patch 
SnakeYAML. You can use it now.
It would be very nice if you can help to extend the documentation to describe 
your case. It would help others to find a quick solution to a similar problem.

Original comment by py4fun@gmail.com on 3 Aug 2010 at 12:44

[GoogleCodeExporter]

It is better to implement project-specific stuff as a plugin (like 
CustomImplicitResolverTest.java) 

Original comment by py4fun@gmail.com on 9 Aug 2010 at 10:36

• Changed state: WontFix