Resolve optional Path correctly

[snejus]

I was adding types to beetsplug/fetchart.py and found that this

@cached_property
def fallback(self) -> Path | None:
    return self.config["fallback"].get(
        confuse.Optional(confuse.templates.Path())
    )

fails with

beetsplug/fetchart.py:1370: error: Incompatible return value type (got "PurePath | None", expected "Path | None")  [return-value]

[github-actions]

Thank you for the PR! The changelog has not been updated, so here is a friendly reminder to check if you need to add an entry.

[jackwilsdon]

Can we not keep this as a PurePath?

Pure paths are useful in some special cases; for example:

1. If you want to manipulate Windows paths on a Unix machine (or vice versa). You cannot instantiate a WindowsPath when running on Unix, but you can instantiate PureWindowsPath.

2. You want to make sure that your code only manipulates paths without actually accessing the OS. In this case, instantiating one of the pure classes may be useful since those simply don’t have any OS-accessing operations.

https://docs.python.org/3/library/pathlib.html

[Copilot]

Pull request overview

This PR fixes the type behavior of confuse.templates.Path when wrapped in confuse.Optional(...), so type-checkers infer pathlib.Path | None (instead of pathlib.PurePath | None)—addressing the reported mypy return-type mismatch.

Changes:

• Update confuse.templates.Path to be typed as Filename[pathlib.Path] (not PurePath).
• Ensure as_template() converts pathlib.PurePath inputs to pathlib.Path before constructing a Path template.
• Add a typing assertion test to validate Optional(Path()) returns pathlib.Path | None.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File	Description
test/test_valid.py	Adds a typing assertion for Optional(Path()), but currently imports typing_extensions in a way that breaks Python 3.13+ test runs.
confuse/templates.py	Adjusts Path’s generic type to pathlib.Path and updates as_template() to pass a concrete pathlib.Path default.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[snejus]

Can we not keep this as a PurePath?

Looking at the following graph

It seems like both PosixPath and WindowsPath inherit from Path. PurePath is too 'stripped down' and generic to be interchangeable with Path. Did a quick lookup on Perplexity: https://www.perplexity.ai/search/c861f2e7-5cb6-4fc0-84b1-833677217bed

[jackwilsdon]

Do we need to use Path in beets or could it be using PurePath? You can always turn a PurePath into a Path by doing Path(pure_path).

It feels more natural to me to have confuse returning an agnostic PurePath (unless I am misunderstanding how it works).

[snejus]

Quoting from the URL I shared:

Core distinction

• PurePath only implements operations that can be done by looking at the path string: joining, normalising, .parent, .name, .suffix, etc. It never performs system calls or inspects the real filesystem.
• Path inherits from PurePath and adds methods that actually hit the OS: .exists(), .is_file(), .is_dir(), .iterdir(), .open(), .unlink(), .mkdir(), .chmod(), .stat(), .cwd(), .home(), etc.

So any time code expects a “real” path that you can read from, write to, or query, a PurePath is not a drop‑in replacement: many of those methods simply don’t exist on PurePath and conceptually shouldn’t, because a pure path is allowed to represent paths that don’t correspond to any real filesystem at all (e.g. Windows paths on a Unix machine).

I think we should assume that users expect to work with 'real' paths?

[jackwilsdon]

Yeah fair enough.

[snejus]

Added another sneaky commit, to make sure that confuse.Path points to confuse.templates.Path instead of pathlib.Path. We should make all imports explicit in the future.