Re: [Django] #33839: Support pathlib.Path in loaders' get_template().

[Django]

#33839: Support pathlib.Path in loaders' get_template().
---------------------------------+-----------------------------------------
Reporter: simon klemenc | Owner: simon klemenc
Type: New feature | Status: assigned
Component: Template system | Version: dev
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 1
Needs tests: 1 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
---------------------------------+-----------------------------------------
Changes (by Mariusz Felisiak):

* needs_docs: 0 => 1
* has_patch: 0 => 1
* needs_tests: 0 => 1

Comment:

[https://github.com/django/django/pull/15835 PR]

--
Ticket URL: <https://code.djangoproject.com/ticket/33839#comment:2>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

[Django]

#33839: Support pathlib.Path in loaders' get_template().
---------------------------------+-----------------------------------------
Reporter: simon klemenc | Owner: simon klemenc
Type: New feature | Status: assigned
Component: Template system | Version: dev
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 1
Needs tests: 1 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
---------------------------------+-----------------------------------------

Comment (by David Smith):

I am not sure we need to support `pathlib.Path`?

In the first example there is:

{{{
os.path.join('adminsortable2', app_label, opts.model_name,
'change_list.html'),
}}}

`os.path.join` returns a `str`. If it wanted to be remove the use of `os`
I think string concatenation using f-string or %-format would also work?
e.g.
{{{
f'adminsortable2/{app_label}/{opts.model_name}/change_list.html'
}}}

Assuming, my above statement is correct. Is there a concrete example where
using `pathlib.Path` over string concatenation is preferable?

I'm also thinking about Jinja2 given the original PR targeted that
backend. It's docs say that the template name should be a `str` or
`Template`. I'm not sure it's Django's responsibility to enhance that API
to do conversion of Paths?
https://jinja.palletsprojects.com/en/3.1.x/api/#jinja2.Environment.get_template.

--
Ticket URL: <https://code.djangoproject.com/ticket/33839#comment:3>

[Django]

#33839: Support pathlib.Path in loaders' get_template().
---------------------------------+-----------------------------------------
Reporter: simon klemenc | Owner: simon klemenc
Type: New feature | Status: assigned
Component: Template system | Version: dev
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 1
Needs tests: 1 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
---------------------------------+-----------------------------------------

Comment (by Carlton Gibson):

TBH I think I'm -1 on this as well.

I see no benefit in loosening the accepted type on the `template_name`
argument, from `str` to `Union[str, Path]` as in the draft PR.
It's a looser API, that doesn't match the underlying expectations, as
David points out.

I'd suggest casting to `str` as a last step if want to use `pathlib` for
the manipulations.

--
Ticket URL: <https://code.djangoproject.com/ticket/33839#comment:4>

[Django]

#33839: Support pathlib.Path in loaders' get_template().
---------------------------------+-----------------------------------------
Reporter: simon klemenc | Owner: simon klemenc

Type: New feature | Status: closed

Component: Template system | Version: dev

Severity: Normal | Resolution: wontfix
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0

Easy pickings: 1 | UI/UX: 0
---------------------------------+-----------------------------------------

Changes (by Mariusz Felisiak):

* status: assigned => closed
* needs_tests: 1 => 0
* needs_docs: 1 => 0
* has_patch: 1 => 0
* resolution: => wontfix
* stage: Accepted => Unreviewed

Comment:

Agreed with David and Carlton.

Simon, please
[https://docs.djangoproject.com/en/stable/internals/contributing/triaging-
tickets/#closing-tickets follow the triaging guidelines with regards to
wontfix tickets] and take this to DevelopersMailingList if you don't
agree.

--
Ticket URL: <https://code.djangoproject.com/ticket/33839#comment:5>

[Django]

#33839: Support pathlib.Path in loaders' get_template().
---------------------------------+-----------------------------------------
Reporter: simon klemenc | Owner: simon klemenc
Type: New feature | Status: closed
Component: Template system | Version: dev
Severity: Normal | Resolution: wontfix
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
---------------------------------+-----------------------------------------

Comment (by Carlton Gibson):

I'll just note, the API contract here isn't to ''paths'' but to ''template
names''. Really it's an implementation detail of the loaders that those
names map to the file system. A DB-based loader (say) would be free to
adopt a totally different naming scheme, such that getting passed
`pathlib.Path` instances would be unexpected, shall we say.

--
Ticket URL: <https://code.djangoproject.com/ticket/33839#comment:6>