Replace path_prefix with vars mapping#46
Conversation
| from pathlib import Path | ||
| import re | ||
| from shutil import rmtree | ||
| from string import Template |
There was a problem hiding this comment.
ho ho -- I think I never used that yet another way to format strings in Python. I only wonder -- why you decided to go for "Template + format" instead of e.g. just making vars to also participate in "format"?
There was a problem hiding this comment.
Because there's no clean way to pull off nested {format} templates.
There was a problem hiding this comment.
not sure I am following on "nested". What I meant is instead of
template2 = Template(path_template).substitute(vars)
return template2.format_map(self.path_fields())have smth like
return template2.format_map(dict(vars, **self.path_fields())There was a problem hiding this comment.
I've changed the implementation to allow using vars via {format} syntax.
| fields = self.path_fields() | ||
| expanded_vars: Dict[str, str] = {} | ||
| for name, template in vars.items(): | ||
| expanded_vars[name] = template.format(**fields, **expanded_vars) |
There was a problem hiding this comment.
right -- I "dreamed" about the bug in my suggested code ;-) this should mitigate it. Thanks!
| class Config(NoExtraModel): | ||
| repo: str | ||
| path_prefix: Optional[str] = None | ||
| vars: Dict[str, str] = Field(default_factory=dict) |
There was a problem hiding this comment.
note to myself -- dict is ordered in any sensible python3 version so order of vars specification matters and could be "relied upon" I guess.
|
@jwodder would you be so kind to add a few tests to ensure that |
|
@yarikoptic Test added. |
Codecov Report
@@ Coverage Diff @@
## master #46 +/- ##
========================================
Coverage ? 2.60%
========================================
Files ? 3
Lines ? 499
Branches ? 74
========================================
Hits ? 13
Misses ? 486
Partials ? 0 Continue to review full report at Codecov.
|
|
looks good, thank you @jwodder |
3 participants
Closes #43.