Implement support for configuration via pyproject.toml #830
Conversation
There was a problem hiding this comment.
Thank you @birdcolour for your PR. I have reviewed it and it looks great - I only have a few remarks (which I can do as a follow-up if you prefer, you will let me know)
| import toml | ||
|
|
||
| with open(pyproject_path, "r") as stream: | ||
| doc = toml.loads(stream.read()) | ||
| if doc.get("tool", {}).get("jupytext") is not None: | ||
| return pyproject_path |
There was a problem hiding this comment.
Don't you think we should have the same lines in get_config_file in the contents manager?
| @@ -343,7 +355,12 @@ def parse_jupytext_configuration_file(jupytext_config_file, stream=None): | |||
| if jupytext_config_file.endswith((".toml", "jupytext")): | |||
| import toml | |||
|
|
|||
| return toml.loads(stream) | |||
| doc = toml.loads(stream) | |||
| print(jupytext_config_file) | |||
There was a problem hiding this comment.
Can we remove the print instruction?
| @@ -477,6 +478,10 @@ def get_config_file(self, directory): | |||
| if self.file_exists(path): | |||
| return path | |||
|
|
|||
| pyproject_path = directory + "/" + PYPROJECT_FILE | |||
There was a problem hiding this comment.
This is the location where I think we should test whether or not there is a jupytext entry in the pyproject file.
(BTW the + "/" is correct, we could add a comment above: paths in Jupyter's contents manager always use the linux path separator, even on Windows)
| @@ -81,6 +84,8 @@ or alternatively, using a dict to map the prefix path to the format name: | |||
| "notebooks/" = "ipynb" | |||
| "scripts/" = "py:percent" | |||
| ``` | |||
| Note that if you are using a `pyproject.toml` file with this dict format, you should make sure the table header is instead `[tool.jupytext.formats]` | |||
There was a problem hiding this comment.
Thanks for documenting this. By the way I was thinking that it would be helpful to give a small example pyproject.toml file, say e.g.
[tool.jupytext]
formats = "ipynb,py:percent"
Where do you think this example would be best located in the documentation? Maybe right after the sample jupytext.tom file with the same configuration?
|
Thank you @birdcolour for your PR, and sorry for taking so long to merge it! I plan to make a new version soon, meanwhile feel free to test the version on the master branch ( |
Resolves #828.
It's slightly annoying that it's necessary to open the
pyproject.tomldocument twice - first to check if it has a valid[tool.jupytext]table infind_jupytext_configuration_file, and then again to parse it properly inparse_jupytext_configuration_file, but I think that rewriting this code to enable a single read would be significantly more complicated, and a rather premature optimisation.Happy to take comments - hopefully the documentation changes read clearly enough.
And once this is done, I'll open a PR here for you :)