Add TOML config loading, autodetect pyproject.toml #374

bollwyvl · 2020-10-31T18:36:12Z

fixes Read DOIT_CONFIG from (pyproject).toml #373

This adds TOML support, with detection of pyproject.toml.

While I like the suggestion on the parent issue of not using SHOUTY keys, I've tried to make it look as much like the existing config format as is reasonable, and we can iterate from there, but figured being able to comment on (maybe broken) code is better than more issue comment traffic.

TODO:

~~there are still some ordering issues... right now, values from doit.cfg would be overwritten by pyproject.toml: need to look into the expected behavior from ConfigParser.~~
- UPDATE: it is indeed FIFO... the merging is still quite naive, e.g. only of the top-level sections, but matches existing behavior
tests
- will it toml?
- will it pyproject.toml?
docs

bollwyvl · 2020-10-31T19:04:13Z

doit/doit_cmd.py

@@ -52,18 +52,28 @@ class DoitMain(object):
                 Strace, TabCompletion, ResetDep)

    def __init__(self, task_loader=None,
-                 config_filenames='doit.cfg',
+                 config_filenames=['pyproject.toml', 'doit.cfg'],


This is to align with the existing behavior of ConfigParser.read, which appears to be FIFO... I verified it, as i didn't see it clearly documented. but makes sense.

Not sure what you mean by FIFO. which file has priority, first or last?

Last-read wins. It seems to be the convention used by isort, pytest, while black only supports pyproject.toml.

bollwyvl · 2020-10-31T19:07:25Z

doit/doit_cmd.py

+            config_filenames = [config_filenames]
+
+        for config_filename in config_filenames:
+            if str(config_filename).lower().endswith('.toml'):


casting to a string, here, as it can be a Path (python 3.6.1+) or bytes (python 3.7).

bollwyvl · 2020-11-15T18:52:55Z

@schettino72 any thoughts here? happy to help hammer out whatever else is missing!

schettino72 · 2020-11-16T13:46:48Z

doc/configuration.rst

@@ -34,6 +32,20 @@ it is processed. It supports 3 kind of sections:
 - a section for each command
 - a section for each plugin category

+.. note::


We are adding first-class support to TOML... so it deserves its own section on docs. Not just a "note".

I would rather "duplicate" the docs than force users to read one and figure out how to translate it...

Also ok to put TOML first and say it is the preferred format (because it might include extra features).

schettino72 · 2020-11-16T13:50:57Z

While I like the suggestion on the parent issue of not using SHOUTY keys, I've tried to make it look as much like the existing config format as is reasonable,

This is not a goal. The goal is to have as idiomatic TOML as possible. I understand you want to keep the code changes to minimum, but we could internally convert TOML format to what whatever the current code is expecting.

bollwyvl · 2020-11-16T17:06:47Z

Great! I'll turn these comments around asap. I'm somewhat rst-illiterate so the docs will be best-effort!

bollwyvl · 2020-11-18T04:00:26Z

I've updated to more closely match the proposed style in #373 (comment). Turns out TOML is rather picky about whitespace around {} so some of the syntax aren't quiet doable (though maybe I need to just learn more).

The translation stuff is multi-step, and perhaps a bit verbose, but seems fine.

I did a sweep of other docs spots, and mostly just duplicated the cfg stuff, as suggested, which didn't seem unreasonable. The multiline strings are nice for, e.g. the auto callbacks.

Think we're getting closer!

bollwyvl · 2021-02-27T14:59:27Z

@schettino72 any thoughts? Happy to do whatever it takes to bring this home!

coveralls · 2021-03-14T12:50:11Z

Coverage decreased (-0.009%) to 99.719% when pulling 0f2b021 on bollwyvl:add-toml into 0aa4cc5 on pydoit:master.

bollwyvl · 2021-03-14T12:54:36Z

I've removed the deprecated pytoml, as the last in-the-wild user of it (flit) has dropped it.

bollwyvl · 2021-03-14T19:20:22Z

All ✔️ again 🎉 !

bollwyvl · 2021-03-23T15:39:20Z

@schettino72 any feedback here?

schettino72 · 2021-03-24T06:26:28Z

@bollwyvl

Last year I was pretty bad at managing my "open-source" time. This year I am targeting 2 hours a week. Note doit is not my only open-source project...

There are still quite a few items before this PR.
If you feel like making a financial contribution I would schedule to work on it on my standard working hours.

Regards

bollwyvl · 2021-03-24T11:12:58Z

No worries there, and do appreciate the timely feedback.

bollwyvl · 2021-03-24T11:48:38Z

I might add, regarding review workload: if there's are additional automation measures that would help reduce your review load, I'm glad to add anything desired to the workflows.

schettino72 · 2021-05-17T17:17:50Z

doc/configuration.rst

+- integers and floating point numbers can be written without quotes
+- boolean values can be written unquoted and lower-cased, as `true` and `false`
+
+Unlike "plain" TOML, `doit` will parse pythonic strings into their correct types,


Why? I think this is in anti-feature. And I could not see where this comes from, all python libs work like this?

This is a hangover from being able to reuse the existing .ini parsing logic, which only speaks string.

oh. i see... I think that is not acceptable.

Welp, that config object (wherever it gets read from or built dynamically) gets handed around throughout the entire running application, it appears, and (only) the cmd_options know how to do the marshaling. It seems like changing (or even detecting the use of) that behavior would require deprecation of the ini format altogether, and a formal data schema throughout all these things... sounds very 1.0ish.

schettino72 · 2021-05-17T17:20:28Z

doc/configuration.rst

+tool.doit
+^^^^^^^^^
+
+The `tool.doit` section may contain command line options that will be used


I guess mentioned this before...

Why not just doit instead of tool.doit? For me it seems unnecessary verbose.

This is specified by PEP 518.

I see it is their recommendation... although I dont really understand/agree why limit to only 2 top tables.

So another question. Why put doit configuration on pyproject.toml instead of going with out own file doit.toml?

Why put doit configuration on pyproject.toml

Indeed, that has a solid question, and mypy has fought back viciously, for some reason. But my feeling is that the more tools I can cram into a single, consensus-driven file, the better I'll be able to automate the creation (through schema, etc), updating, etc. vs code.

doit.toml?

A fine idea as well, and could be added to the list. I'd imagine this one wouldn't have the tool.doit prefix.

schettino72 · 2021-05-17T17:22:37Z

doit/doit_cmd.py

@@ -52,18 +52,28 @@ class DoitMain(object):
                 Strace, TabCompletion, ResetDep)

    def __init__(self, task_loader=None,
-                 config_filenames='doit.cfg',
+                 config_filenames=['pyproject.toml', 'doit.cfg'],


Not sure what you mean by FIFO. which file has priority, first or last?

schettino72

Sorry for delay. Here is another round of questions...

schettino72 · 2021-05-17T17:27:07Z

doc/configuration.rst

+   [tool.doit.tasks.make_cookies]
+   cookie_type = "chocolate"
+   temp = "375F"
+   duration = 12



You mentioned about being able to define tasks in TOML, right?

Have you thought about what the name would be? I would thing doit.tasks. So maybe get a different name for setting task parameters doit.params.<task_name>, what do you think?

I would thing doit.tasks. So maybe get a different name for setting task parameters doit.params.<task_name>, what do you think?

Again, the naming is just trying to keep the new format as parallel as possible while both config formats are supported.

I don't recall ever actually having used the task params, but I'm sure someone would want to! If anything, I'd imagine putting everything this PR has done thus far under tool.doit.config, leaving the top-level namespace clear for things like tasks....

You mentioned about being able to define tasks in TOML, right?

I had imagined this as a separate loader, e.g. TomlLoader which could be enabled via pyproject.toml#/tool/doit/(config)/loader which would then read pyproject.toml (again) and actually populate tasks. The toml loader could default to pyproject.toml, but, lacking inheritance, etc. it seems like a project might want paths = [path="pyproject.toml", ref="tool.doit.tasks"].

For the actual task definitions, I'm imagining the high road would, whether such a loader was part of this repo or standalone:

define and ship a JSON schema stored as JSON, (though built from TOML, perhaps, for readability)

ideally, this would support some measure of extensibility, for, e.g. config_changed without going full YAML tag nonsense (which TOML won't support anyway)

some other things are nasty, e.g.

enumerating files

maybe have a paths definition section which can use something like pathspec, and which tasks can reference by pointer

create_after

pick a default place in pyproject.toml, e.g. #/tool/doit/tasks or #/tool/doit-toml-tasks

optionally enable pre-validation (jsonschema is not exactly lightweight, as a dependency or a runtime)

schettino72 · 2021-05-17T17:30:46Z

doc/extending.rst

+.. code-block:: toml
+
+   [tools.doit.plugins]
+   COMMAND = { foo = "my_plugins:FooCmd" }


what about?

[doit.plugins.foo] kind = "command" obj = "my_plugins:FooCmd"

Better name for "obj" ?

yeah, naming things is hard. If it was purely data, the more verbose form would feel good, but i think in this case spareness wins.

I believe this could also be written

[tool.doit.plugins.COMMAND] foo = "my_plugins:FooCmd"

(have updated docs to reflect both)

codecov-commenter · 2021-05-17T18:11:46Z

Codecov Report

Merging #374 (cff267c) into master (c5bd59a) will decrease coverage by 0.00%.
The diff coverage is 98.41%.

@@            Coverage Diff             @@
##           master     #374      +/-   ##
==========================================
- Coverage   99.72%   99.71%   -0.01%     
==========================================
  Files          65       65              
  Lines        8852     8912      +60     
==========================================
+ Hits         8828     8887      +59     
- Misses         24       25       +1

Impacted Files	Coverage Δ
doit/doit_cmd.py	`99.28% <97.43%> (-0.72%)`	⬇️
tests/test_doit_cmd.py	`100.00% <100.00%> (ø)`

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c5bd59a...cff267c. Read the comment docs.

schettino72 · 2021-11-14T07:00:36Z

Hey, I finally have merged this. It would be great if you could do one more pass reviewing this.

I went with lowercase version like:

[tool.doit.plugins.command]
foo = "tests.sample_plugin:MyCmd"

I also added support to loading from TOML file without tool.doit.
Just realized command line there is no way specify a config file, so it is only available through API call for now.

Thanks

bollwyvl · 2021-11-14T14:19:50Z

Sounds great, thanks!

bollwyvl added 3 commits October 31, 2020 14:26

add (pyproject).toml config parsing

af76101

add packaging/dev metadata stuff for toml

596c7fd

fix config file ordering, handle Paths

42ad151

bollwyvl commented Oct 31, 2020

View reviewed changes

update config docs

09aba9e

bollwyvl changed the title ~~[wip] Add TOML config loading, autodetect pyproject.toml~~ Add TOML config loading, autodetect pyproject.toml Oct 31, 2020

schettino72 reviewed Nov 16, 2020

View reviewed changes

bollwyvl added 3 commits November 17, 2020 22:30

add mapping from proposed TOML to ConfigParser, more docs

dcf9f6e

hoist import detection, remove some early returns

caac79f

more docs tweaks

e678632

add tool.doit canary check, warning with supported libs

2fb5f80

bollwyvl closed this Mar 14, 2021

bollwyvl reopened this Mar 14, 2021

Merge remote-tracking branch 'upstream/master' into add-toml

fca5cc3

remove deprecated pytoml (no longer used by flit)

0f2b021

Merge remote-tracking branch 'upstream/master' into add-toml

615edd1

schettino72 reviewed May 17, 2021

View reviewed changes

merge upstream

87cd2b3

schettino72 reviewed May 17, 2021

View reviewed changes

fix tools vs tool in docs

cff267c

schettino72 closed this Nov 14, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add TOML config loading, autodetect pyproject.toml #374

Add TOML config loading, autodetect pyproject.toml #374

bollwyvl commented Oct 31, 2020 •

edited

Loading

bollwyvl Oct 31, 2020

schettino72 May 17, 2021

bollwyvl May 17, 2021

bollwyvl Oct 31, 2020

bollwyvl commented Nov 15, 2020

schettino72 Nov 16, 2020

schettino72 commented Nov 16, 2020

bollwyvl commented Nov 16, 2020 via email

bollwyvl commented Nov 18, 2020

bollwyvl commented Feb 27, 2021

coveralls commented Mar 14, 2021 •

edited

Loading

bollwyvl commented Mar 14, 2021

bollwyvl commented Mar 14, 2021

bollwyvl commented Mar 23, 2021

schettino72 commented Mar 24, 2021

bollwyvl commented Mar 24, 2021 via email

bollwyvl commented Mar 24, 2021

schettino72 May 17, 2021

bollwyvl May 17, 2021

schettino72 May 17, 2021

bollwyvl May 17, 2021

schettino72 May 17, 2021

bollwyvl May 17, 2021

schettino72 May 17, 2021

bollwyvl May 17, 2021

schettino72 May 17, 2021

schettino72 left a comment

schettino72 May 17, 2021

bollwyvl May 17, 2021

schettino72 May 17, 2021

bollwyvl May 17, 2021 •

edited

Loading

codecov-commenter commented May 17, 2021

schettino72 commented Nov 14, 2021

bollwyvl commented Nov 14, 2021 via email

Add TOML config loading, autodetect pyproject.toml #374

Add TOML config loading, autodetect pyproject.toml #374

Conversation

bollwyvl commented Oct 31, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bollwyvl commented Nov 15, 2020

Choose a reason for hiding this comment

schettino72 commented Nov 16, 2020

bollwyvl commented Nov 16, 2020 via email

bollwyvl commented Nov 18, 2020

bollwyvl commented Feb 27, 2021

coveralls commented Mar 14, 2021 • edited Loading

bollwyvl commented Mar 14, 2021

bollwyvl commented Mar 14, 2021

bollwyvl commented Mar 23, 2021

schettino72 commented Mar 24, 2021

bollwyvl commented Mar 24, 2021 via email

bollwyvl commented Mar 24, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

schettino72 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bollwyvl May 17, 2021 • edited Loading

Choose a reason for hiding this comment

codecov-commenter commented May 17, 2021

Codecov Report

schettino72 commented Nov 14, 2021

bollwyvl commented Nov 14, 2021 via email

bollwyvl commented Oct 31, 2020 •

edited

Loading

coveralls commented Mar 14, 2021 •

edited

Loading

bollwyvl May 17, 2021 •

edited

Loading