-
-
Notifications
You must be signed in to change notification settings - Fork 797
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
Add StreamReadCapability.EXACT_FLOATS
to indicate whether parser can expose exact floating-point values or not
#733
Conversation
7e2c173
to
db76643
Compare
db76643
to
aaa3223
Compare
Hmmh. I'll have to think about this some more; although the "yes, got exact value" part makes sense, the part about copying numbers as Strings is trickier (it might also make sense to just use My specific concern is whether logic makes sense for all textual backends -- YAML, for example, exposes all kinds of exotic formats -- and then re-parsing might not work by databind, nor possible cases of copying from But one way we could also fix the JSON part, I think, might be to selectively override functionality in Further, it probably would at least make sense to further refactor Going back to the original problem of only some back-ends allowing textual number writes... there actually IS another matching
which should help figure out if writing works. So maybe logic would check...
I am not quite sure how exactly this should work, I guess -- does YAML allow exotic double presentations (for integers there's the whole menagerie of base-N variants, but for FPs?) -- but I hope above gives some more ideas. One thing I'd be happy to do first, though, would be to do one PR only adding the new |
Oh, one other thing: @htmldoug have I asked for CLA yet? If not, that's one thing needed before I can merge a PR. https://github.com/FasterXML/jackson/blob/master/contributor-agreement.pdf and the usual way is to print it, fill & sign, scan/photo, email to Looking forward to getting the new capability merged! |
CLA received. So, at this point, I'd like to get a smaller PR for the new capability, get that (those, for dataformats) merged in, and then consider how to implement actual logic. |
Thanks for the prompt CLA receipt confirmation and thoughtful replies. Descoped as requested. Would you like for me to include the failing FWIW, the new Thanks for the heads up on I don't see any dataformat PRs tagged to #630, so maybe flag usage just hasn't been rolled out yet. Sounds we could use an audit so we'll be able to take advantage of this someday. Are there any shared compliance tests where we could add something like: if (CAN_WRITE_FORMATTED_NUMBERS) then roundtrip |
@htmldoug New tests would be great -- I put those under "src/test/java/..../failing" instead of I hope to look into this tomorrow or day after, and get this one merged; will respond to other questions too. Happy New Year! |
Cool. I've moved the To be clear, I've got a workaround in place, so I'm good for now. No rush! A fix would let me remove that workaround, but there's no urgency from my end. I'm just here to help out and give back if I can. Happy new year! |
StreamReadCapability.EXACT_FLOATS
to indicate whether parser can expose exact floating-point values or not
Merged. |
Defaulting for the new capability introspection now added to binary format backends (ones in |
Adds
StreamReadCapability.EXACT_FLOATS
as suggested in #730 (comment).