python-poetry / tomlkit

Style-preserving TOML library for Python
MIT License
693 stars 98 forks source link

Support `Decimal` and `MutableMapping` codecs, add a multiline-`Array` indent control #294

Open goodboy opened 1 year ago

goodboy commented 1 year ago

In an attempt to resolve the feature request issues i submitted:


Summary:


Possibly still todo?

goodboy commented 1 year ago

Bah, going through rebase to master 🏄🏼

goodboy commented 1 year ago

There hopefully i got all the latest changes in now B)

goodboy commented 1 year ago

Also, if this patch is likely to never be accepted or needs to be changed to be accepted please let me know asap since we'd much rather not have to maintain our own fork :)

frostming commented 1 year ago

a better way to support this and similar cases is to allow passing a custom encoder to item() function.

i don't think we need round trip

goodboy commented 1 year ago

@frostming ah ok, I'm fine to change it to do that.

(also sorry i missed your comment don't have my email hooked up rn for GH 😂)

is to allow passing a custom encoder to item() function.

can you show an example that might already exist for doing this btw in the code base and for which feature addition out of the issues I made does this apply? All?


i don't think we need round trip

Also can you clarify wym by that?

frostming commented 1 year ago

You can convert a decimal to a float representation in TOML, but can't convert it back, and Decimal isn't listed as the data types defined by TOML spec, so it's not worth to expose a new api and item type for it.

Instead, I'd propose something like this:

import tomlkit
a = Decimal('34.56')

def my_encoder(o):
    if isinstance(o, Decimal):
        return tomlkit.float_(o)
    raise ValueError(f"Invalid type {type(value)}")

item = tomlkit.item(a, encoder=my_encoder)   # a Float object
v = {"a": Decimal("34.56")}
print(tomlkit.dumps(v, encoder=my_encoder)
# a = 34.56

In this way users can convert custom types, not only Decimal, to TOML items in tomlkit. And it is clearer that users can't convert it back(parse anything as Decimal)

goodboy commented 1 year ago

so it's not worth to expose a new api and item type for it. In this way users can convert custom types, not only Decimal, to TOML items in tomlkit. And it is clearer that users can't convert it back(parse anything as Decimal)

@frostming works for me. So I guess i'll just drop all the Decimal related changes then and rebase this branch a bit.

Any other quiffs and/or do you have any suggestions for the tests I still need to do?

frostming commented 1 year ago

@goodboy yes, I've proposed a change in #296

goodboy commented 1 year ago

@frostming thanks for the follow up!

i will hopefully get back to this PR this week. Will check out that landed PR and change our stuff to match first, then come back here and drop the unneeded history.

Still kinda looking for a little guidance on how thorough the new tests for the multi-line arrays should be? i have some basic ideas but don't want to push too deep if y'all aren't that concerned about it.