Skip to content
This repository has been archived by the owner on Feb 27, 2020. It is now read-only.

bad outputs for f2s #4

Closed
timotheecour opened this issue Feb 12, 2020 · 4 comments
Closed

bad outputs for f2s #4

timotheecour opened this issue Feb 12, 2020 · 4 comments
Labels
wontfix This will not be worked on

Comments

@timotheecour
Copy link
Contributor

timotheecour commented Feb 12, 2020

I know it's still WIP, but just for reference:

  template test2(x, s) =
    let s2 = f2s(x)
    check s2 == s
  test2 0.0, "0.0" # current: 0e0
  test2 1.0, "1.0" # current: 1e0
  test2 1.3, "1.3" # current: 1.3e0
  test2 0.12345678901234567890123456789, "0.12345678901234568" # should truncate to same as python3, I think; current: 1.2345679e-1 which doesn't have enough digits
  • some of these are real errors (not roundtrip safe), like 0.12345678901234567890123456789
  • some of these are cosmetic errors; eg e0 should just be stripped
  • the .0 shouldn't be stripped before an exponent

that's for consistency with almost all languages; if needed i can do some more research to double check what's printed in other langs

@disruptek
Copy link
Owner

This is not a printf format code; it's for stringifying a float.

@disruptek disruptek added the wontfix This will not be worked on label Feb 12, 2020
@disruptek
Copy link
Owner

It's easiest if you just PR the tests; you can do that right from the web interface.

@timotheecour
Copy link
Contributor Author

timotheecour commented Feb 12, 2020

It's easiest if you just PR the tests; you can do that right from the web interface.

I can add a PR with failing tests, sure; obviously would have to use when/if false since they'd fail...

This is not a printf format code; it's for stringifying a float.

well in my mind this repo should eventually (looks like this could be soon given what you already have) replace system.addFloat, which is needed to properly close nim-lang/Nim#13196 (among other benefits) (and araq seems to agree with that direction see nim-lang/Nim#13364 (comment))
Obviously there are still things to figure out for this to work (and be importable in system without bloating dependencies; which is feasible via a magic IMO), but if that's (one of) the goals, f2s should be suitable for that. It doesn't have to be the only format option though, and I think f2s could get a format string (with maybe few initial supported options, subset of what printf options allow, to grow as needed over time) eg:

f2s(result: var string, x: float, fmt: string) = ...
# or maybe via static string if benchmarking shows it can affect performance in any way
f2s(result: var string, x: float, fmt: static string) = ...

doAssert f2s(1.2) == "1.2" # implies default fmt = "%g"
doAssert f2s(0.12, "%f") == "1.2e-1"
doAssert f2s(0.12, "%e") == "0.12"
doAssert f2s(0.12, "%g") == "0.12" # Use the shortest representation: %e or %f

@disruptek
Copy link
Owner

If they don't fail, then the tests aren't useful.

There's a printf component to Ryu that we can port for printf-like behavior that is consistent with Ryu itself. This would be suitable for adoption in addFloat, etc.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
wontfix This will not be worked on
Projects
None yet
Development

No branches or pull requests

2 participants