[WIP][RFC] Add TOML #32
Conversation
Codecov Report
@@ Coverage Diff @@
## master #32 +/- ##
==========================================
- Coverage 99.81% 99.27% -0.54%
==========================================
Files 9 9
Lines 536 553 +17
==========================================
+ Hits 535 549 +14
- Misses 1 4 +3
Continue to review full report at Codecov.
|
Codecov Report
@@ Coverage Diff @@
## master #32 +/- ##
==========================================
- Coverage 99.81% 99.27% -0.54%
==========================================
Files 9 9
Lines 536 553 +17
==========================================
+ Hits 535 549 +14
- Misses 1 4 +3
Continue to review full report at Codecov.
|
This does not handle mapping to the types TOML has over JSON
| closed_on = "17:30:00" | ||
| customers = 192 | ||
| day_type = "Holiday" | ||
|
|
There was a problem hiding this comment.
To the best of my knowledge, TOML also supports the following data types that are from missing from this test.
Boolean
Inline Table
Vanilla Arrays
Nested Arrays of tables
There was a problem hiding this comment.
We reviewed your PR as a training exercise. We recognize this was meant to be a minimal TOML add on and to that end it looks awesome! Congrats!
I thought I'd quickly add TOML as an optional dependency to see how it fits in, as the interface is like the usual JSON+YAML load(s)/dump(s). It was easy to drop in TOML as a store for objects, but that's without any mapping to additional types in TOML.
Mapping to TOML types would be nice, as it could allow more generally craftable/ingestable toml files.
I'm thinking there are some options (in order of personal preference):
to_dict(…)single dispatch methods to take something like afor_toml=Truekwarg which branches the handling. I'm not sure how this would tie in withto_modelother than making the relevant fields fields also handlerelatedat the momentAre there any maintainer/community thoughts on this?