Repository navigation
Conversation
…h that does not
All three functions take `output_dir`; the docstrings call it `output_path` and then promise
that "if None, the rendered template is only returned".
Neither half holds:
- the parameter is `output_dir`, and `_render_template` returns `None` — the rendered text is
written to a file, never returned;
- passing `None` does not fall back to anything. `render_base_config` does
`os.path.join(output_dir, "base.yaml")`, which raises
TypeError: expected str, bytes or os.PathLike object, not NoneType
So the documented "optional" behaviour is a crash. The docstrings now describe what the
functions do.
Docstrings only: AST comparison against the previous head is identical once docstrings are
excluded, and the module compiles.
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
All three functions in this file take
output_dir. The docstrings call itoutput_path, and then promise that "if None, the rendered template is only returned".Neither half holds:
output_pathoutput_dir_render_templatereturnsNone; the text goes to a filerender_base_configdoesos.path.join(output_dir, "base.yaml"), which raisesSo the documented "optional" behaviour is a crash, not a fallback. The docstrings now say what the functions do.
Verification
python -m py_compileon the module: passes.TypeErrorabove is run, not inferred.Note:
output_dir: Optional[str] = Noneis still in the two signatures. I did not change the signatures — the annotation and the body disagree, but that is a code decision, not a docstring one.