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

Function to convert everything to plain old python objects #43

Closed
clbarnes opened this issue Mar 14, 2019 · 11 comments · Fixed by #187
Closed

Function to convert everything to plain old python objects #43

clbarnes opened this issue Mar 14, 2019 · 11 comments · Fixed by #187

Comments

@clbarnes
Copy link

As more projects incorporate PEP518, many will be aiming to keep compatibility with their existing config files as well (be they ini, yaml, json, etc.). In order to help abstract over config files of different types, it would be useful to be able to convert tomlkit objects into POPOs. One of TOML's strengths for python is that all of its constructs can deserialise to POPOs: could this be made available for tomlkit users?

@tmose1106
Copy link

As far as I can tell, this can be achieved using the .value method. You can look at the source for it here.

@jasongraham
Copy link

That seems to unwrap all of the Container objects, but several of the Item objects that inherit from the built in types (int, str, float) have a .value method that returns self rather than their built in Python equivalents.

@LexiconCode
Copy link

LexiconCode commented Jan 9, 2020

That seems to unwrap all of the Container objects, but several of the Item objects that inherit from the built in types (int, str, float) have a .value method that returns self rather than their built in Python equivalents.

Apparently loading with .value can tomlkit still uses its own classes to represent real numbers which is undesirable. Do you have any thoughts on this @sdispater ?

@cash
Copy link

cash commented Apr 7, 2020

The lack of an ability to easily get plain old python objects has caused me to not use tomlkit. I don't want to have to write wrapper code to convert everything. The library should make that easy.

@ziotom78
Copy link

I have implemented the following function, which works for most of the cases. @sdispater , if you're interested I can make a PR.

def tomlkit_to_popo(d):
    try:
        result = getattr(d, "value")
    except AttributeError:
        result = d

    if isinstance(result, list):
        result = [tomlkit_to_popo(x) for x in result]
    elif isinstance(result, dict):
        result = {
            tomlkit_to_popo(key): tomlkit_to_popo(val) for key, val in result.items()
        }
    elif isinstance(result, tomlkit.items.Integer):
        result = int(result)
    elif isinstance(result, tomlkit.items.Float):
        result = float(result)
    elif isinstance(result, tomlkit.items.String):
        result = str(result)
    elif isinstance(result, tomlkit.items.Bool):
        result = bool(result)

    return result

@dbohdan
Copy link

dbohdan commented Sep 15, 2020

You can find a converter that processes TOML Kit's dates and time classes in remarshal.

@syntapy
Copy link
Contributor

syntapy commented Apr 12, 2022

I'm working on this right now. Should I make it so that the .value property returns a popo? Or should I add a method such as .as_popo to each relevant class?

@syntapy
Copy link
Contributor

syntapy commented Apr 12, 2022

So it looks like the .value property of a tomlkit.items.Float or a tomlkit.items.Integer returns a tomlkit item, while for a tomlkit.items.Bool it returns a popo boolean

And theres some strange stuff going on with tomlkit.items.Array. If I create a python popo list with a list_tomlkit = [tomlkit.items.Bool instance, tomlkit.items.String instance], and then create the Array from that with array_tomlkit = tomlkit.items.Array(list_tomlkit, trivia), then when I do array_tomlkit[0] it shows it as a popo, even though in list_tomlkit its a tomlkit obj

So it seems like theres a bit of inconsistency in that aspect. I'd be happy to fix that

@clbarnes
Copy link
Author

clbarnes commented Apr 12, 2022

It feels like .value is a good place for it, although it's not necessarily clear whether that is/ should be recursive for container types, and changing the existing functionality always has the chance of breaking workflows. Maybe something like a .unwrap(recursive=True), method would give it a clean start. Or to_native, to_builtin, to_stdlib; "popo" has been useful for discussing this issue but I'm not sure it's very commonly used in python generally.

@syntapy
Copy link
Contributor

syntapy commented Apr 12, 2022

changing the existing functionality always has the chance of breaking workflows

Would it be reasonable for me to add a warning in each call to .value that that method will be phased out in a future version? And that they should change to .unwrap before that?

Edit: Should I make it only show x number of warnings so as not to overload them?

@clbarnes
Copy link
Author

I think that .value currently does what it was intended for. While it does return tomlkit types, those types are generally subclasses of popos and so basically any definition of duck typing would call that fine. Additionally, I'd say we should be careful about changing .value too much because a few of the tomlkit types are enums, which have their own implementation.

Anyone currently using .value won't want additional warnings, and I don't think it needs to be phased out at all. Anyone starting to use the library should just use the new method, which will hopefully be well documented. I'd maybe just change the docs of the .value property to be clear that it doesn't necessarily return the wrapped object.

One more decision to make is whether Trivia, Whitespace etc. should have this new method at all, or whether they should have it and return some singleton which the caller needs to deal with (can't use None for obvious reasons).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants