Skip to content
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

Allow truncation in %.f format specifier #73

Closed
sgoll opened this issue Aug 3, 2024 · 5 comments · Fixed by #74
Closed

Allow truncation in %.f format specifier #73

sgoll opened this issue Aug 3, 2024 · 5 comments · Fixed by #74
Labels
bug Something isn't working

Comments

@sgoll
Copy link

sgoll commented Aug 3, 2024

Description

Currently, using %.f format specifiers in strftime() allows setting the minimum amount of digits to use, e.g. %.3f, as stated in the docs:

… print a fractional second component to at least 3 decimal places.

This is fine and indeed the useful approach for most use-cases. However, it could be nice to be able to also limit the maximum number of digits. Right now, this can only be achieved by rounding the value before printing it, e.g. round(jiff::Unit::Millisecond).

@BurntSushi BurntSushi added the bug Something isn't working label Aug 3, 2024
@BurntSushi
Copy link
Owner

I think this is a bug. My intent was that %.3f would also limit to 3 decimal places. But indeed, that's not the current behavior. This new test:

        insta::assert_snapshot!(f("%.3f", mk(123_456_000)), @".123");

Fails:

Expression: f("%.3f", mk(123_456_000))
─────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────
-old snapshot
+new results
────────────┬────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────
    0       │-.123
          0 │+.123456
────────────┴────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────

BurntSushi added a commit that referenced this issue Aug 3, 2024
Previously, we were formatting fractional components of
numbers by kinda abusing the DecimalFormatter. But in
order to fix #73, I found it simpler to split the
fractional case out of the "normal" number formatting.

This commit just adds the fractional formatter. Subsequent
commits will migrate things to it.
BurntSushi added a commit that referenced this issue Aug 3, 2024
Previously, formatting 1.123456789 seconds with %S%.3f would
result in 1.123456789, but it should be 1.123. This commit fixes
that by switching to the new fractional formatter in the previous
commit.

Fixes #73
BurntSushi added a commit that referenced this issue Aug 3, 2024
Previously, formatting 1.123456789 seconds with %S%.3f would
result in 1.123456789, but it should be 1.123. This commit fixes
that by switching to the new fractional formatter in the previous
commit.

Fixes #73
BurntSushi added a commit that referenced this issue Aug 3, 2024
Previously, we were formatting fractional components of
numbers by kinda abusing the DecimalFormatter. But in
order to fix #73, I found it simpler to split the
fractional case out of the "normal" number formatting.

This commit just adds the fractional formatter. Subsequent
commits will migrate things to it.
BurntSushi added a commit that referenced this issue Aug 3, 2024
Previously, formatting 1.123456789 seconds with %S%.3f would
result in 1.123456789, but it should be 1.123. This commit fixes
that by switching to the new fractional formatter in the previous
commit.

Fixes #73
@BurntSushi
Copy link
Owner

I fixed this in #74. I'll do a release this weekend at some point.

Note that this will only do truncation. If you need non-truncation rounding, you'll have to keep doing what you're doing. Formatting can't really feasibly do anything other than truncation because rounding can increase other units. The cases are probably pathological, but for example, doing %Y %.0f with 2024-12-31T23:59:59.999999999Z would need to increment the year when using the HalfExpand rounding mode.

My bet though is that truncating at the sub-second level is probably good enough for most use cases. You might consider doing that in your GraphQL integration since rounding can be a bit heavy-weight. (Rounding requires normalizing the date/time representation, doing the rounding and then un-normalizing back.)

@sgoll
Copy link
Author

sgoll commented Aug 9, 2024

I fixed this in #74. I'll do a release this weekend at some point.

When will you make the next release? I think, the Jiff integration in Juniper can be merged once I make the changes that will be made possible by the upcoming Jiff release, cf. graphql-rust/juniper#1271 (comment).

@BurntSushi
Copy link
Owner

Right, yeah, it slipped. I'll do one today.

@BurntSushi
Copy link
Owner

Okay, jiff 0.1.5 is now out!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants