Bad %d conversion?

[leres]

I started with:

n = delta.seconds / 3600
msg = '%s last updated %d hour%s ago' % (path, n, plural(n))

flynt converted this to:

msg = f'{path} last updated {n:d} hour{plural(n)} ago'

which results in the error:

ValueError: Unknown format code 'd' for object of type 'float'

To be fair the calculation of "n" should have used integer division (//) but the conversion shouldn't break the script.

[ikamensh]

this is unfortunate that old % formatting has a different behavior, https://stackoverflow.com/questions/5627605/string-formatting-with-0d-format-gives-unknown-format-code-d-for-object-o

Example:

abc = 12.23

print("%d" % abc)  # works, prints 12
print(f"{abc:d}")  # raises

This leaves the choices:

1. detect this situation and skip conversion
2. change the behavior of the code silently - remove %d, therefore the strings we are creating will be different
3. convert and keep %d (current option) - as you are showing it has a risk of creating exceptions. Good part about exceptions is that you are much more likely to notice them fast, unlike option 2) where you might wonder why some tool parsing output of your software will stop working.

[leres]

I think the root problem here is that under the hood, f-strings use str.format() under the hood. For example, this works:

n = 1.5
'n is %d' % n

and these fails:

'n is {n:d}'.format(n=n)
f'n is {n:d}'

So really, any conversion flynt does of %d to :d has the potential to introduce a new exception.

I can think of a 4th option: convert and keep %d but also wrap the value with int():

f'n is {int(n):d}'

This keeps % formatting semantics and prevents flynt from breaking working code. You might emit a warning that the int() might not be necessary. Or at the least document the considerations discussed here.

I think adding int() is preferable to taking the risk of creating exceptions. There will be strings that are only formatted when something goes wrong and I really hate it when an error print() is broken and I miss out on the hint needed to fix the underlying problem. You can argue that testing will reveal problems but the reality is testing is not universal.

[ikamensh]

int(n) has yet different behavior from %d formatting (e.g. it could accept a string '123'). I think a thorough solution will involve a new flag, --aggressive, and for False flynt will skip %d formatting. For True it will replace it either with no specifier ("%d" % a -> f"{a}") or wrap result in int() call.

This will need changes in CLI interface and tests as well, so ETA for this change is couple of months, I don't have much time to work on this now. In the meanwhile will add a section about %d to README # Dangers of conversion.

[ikamensh]

fixed in 0.47