Support passing pathlib.Path in some functionalities #1499
Conversation
| """ | ||
|
|
||
| if isinstance(root, Path): | ||
| root = str(root) |
There was a problem hiding this comment.
You can skip the type check and just call str(var) or Path(var) to get what you need. This may even be cheaper than the type check :)
There was a problem hiding this comment.
Hi @defnull, thank you for your review!
I have a concern about just calling str(var) on it. If someone passes an invalid type here, like None, the value will become a string literal "None", and it may not fail here, but fail eventually later in the codebase somewhere else. So I think it's better to just handle the expected type here, to reduce the risk of this kind of error.
| self.source = source.read() if hasattr(source, 'read') else source | ||
| self.filename = source.filename if hasattr(source, 'filename') else None | ||
| self.lookup = [os.path.abspath(x) for x in lookup] if lookup else [] | ||
| if lookup: |
There was a problem hiding this comment.
Same here, just call str on each value unconditionally. The check for an empty lookup variable could also be skipped, as a list comprehension over an empty iterator results in an empty list. This is not a hot path, so a little bit of overhead is fine, readability is more important.
There was a problem hiding this comment.
Unfortunately, the default value of lookup is None, because of the common pitfall of Python's mutable types as default parameters. So we have to check if lookup is a falsy value here.
2 participants
Related to #1311, Support using
pathlib.Pathin:load_configstatic_filelookupfields inBaseTemplate