Returning Path instead of str instances

[WhyNotHugo]

As mentioned in ActiveState/appdirs#79, I've been working on another fork of appdirs, but would rather hold off on publishing that and focus all efforts in one place.

I mostly have two changes in places:

• Drop support for ancient python versions (there's little point in a new library supporting Pythons that re long past their EOL, and that just holds back on keeping the library alive).
• Change the return type of directories to Path instance, to make usage on consumer libraries easier. This kinda means it's not a 100% drop-in replacement, but rather an upgrade (e.g.: like a major).

Maybe it would make sense to have a platformdirs==1.0.0 release, which is functionally identical to appdirs (to allow seamless switching), and then start merging in any non-backwards-compatible changes. Thoughts?

[YariKartoshe4ka]

1. I absolutely agree with @WhyNotHugo that we need to stop supporting EOL versions of Python because it does not make sense
2. If we drop old Pythons support, it is logical to do everything in a modern way and follow PEP428
3. I'm not sure about platformdirs ==1.0.0, because user can still use the original appdirs (judging because the project is frozen, we should not worry about it)

[WhyNotHugo]

I'm not sure about platformdirs ==1.0.0, because user can still use the original appdirs (judging because the project is frozen, we should not worry about it)

You're right. No need to have a half-migration path.

[Julian]

virtualenv depends on appdirs (and as soon as we'd have a release, presumably on this module) and still supports 2.7, so we can't drop support for 2.7 until a release happens I think, and then we're free to presumably.

(The pedant in me will also mention that there are versions of 2.7 that are not EOL'ed, i.e. PyPy, but certainly if it causes even trivial extra effort, like requiring us to depend on pathlib2 for 2.7, I think at this point it's totally fine to drop after a release).

And as for the specific idea, yeah, returning Path objects seems like a good idea to me.

[WhyNotHugo]

I'd never heard of pathlib2, but a quick lookup indicates it's unsupported since 2020-01-01:

As of January 1 2020, this repository will no longer receive any further updates, as Python 2 is no longer supported.

[WhyNotHugo]

The order of the two items in my original message is by design: dropping those versions is a blocker to adopting Pathlib. Python 3.6 also have some additional changes.

I believe that pre-3.6, open(Pathlib("/some/file")) did not work.

The key difference is, applications would fail on python3.5 due to an API very inconsistent with appdirs, but not on later versions. Seems like a very subtle for downstreams to figure out.

[YariKartoshe4ka]

If I understood correctly, then we should make a release with the support of all Python (which virtualenv supports) to switch virtualenv to us, and then drop all EOL Pythons (include PyPy) and add support for pathlib? Or we will still have compatibility with EOL Pythons?

[WhyNotHugo]

That makes sense to me.

I'm curious on whether virtualenv plans to drop python2. I haven't some any reference in the docs or their issue tracker.

[gaborbernat]

As long as there's one python 2 compatible release virtualenv will be fine. virtualenv plans to support Python versions two years past their grace period - https://gaborbernat.github.io/euro_python_2019/#/55; so will not drop Python 2 support until January 1st. But yeah PyPy2 support on virtualenv side might hold back virtualenv on Python 2 land for longer...

[RonnyPfannschmidt]

providing a path type aware wrapper might be nice

so one could get a Appdirs(...., path_type=pathlib.Path/pathlib2.Path)

[gaborbernat]

so one could get a Appdirs(...., path_type=pathlib.Path/pathlib2.Path)

That sounds a lot like https://adamj.eu/tech/2021/07/10/python-type-hints-how-to-avoid-the-boolean-trap/, I think it's better to have new method names for the path variant 😊

[papr]

@gaborbernat So, when it comes to naming the new methods, what do you think about adding a _as_path suffix?

user_data_dir_as_path
user_config_dir_as_path
user_cache_dir_as_path
user_state_dir_as_path
user_log_dir_as_path
site_data_dir_as_path
site_config_dir_as_path

Also, do I see it correctly, that the implementation would just need to wrap their str-counterpart in pathlib.Path?

[gaborbernat]

user_data_path
user_config_path
user_cache_path
user_state_path
user_log_path
site_data_path
site_config_path

And yes.

[WhyNotHugo]

@gaborbernat Those look great! Was gonna mentioned the same thing :)

I intended to eventually handle Paths completely internally (e.g.: Path.home() / ".cache"). I guess that if the old methods are kept around then internally everything will continue to be handled as strings?

[papr]

I would sit down later today and implement these. @WhyNotHugo I do not think there is much benefit of changing the internal representation. IIRC using pathlib can also be slower in some cases? Not sure about this, though. If I am wrong, and there are good arguments for changing the internal representation, I would be happy to give that a go, too.

[WhyNotHugo]

It probably doesn't make sense to change it internally if the `str` methods will remain. Though I'm honestly very curious how both implementations would compare in terms of performance.

[gaborbernat]

@gaborbernat Those look great! Was gonna mentioned the same thing :)

I intended to eventually handle Paths completely internally (e.g.: Path.home() / ".cache"). I guess that if the old methods are kept around then internally everything will continue to be handled as strings?

No, the str variant is the faster. That remains the internal default form.

[papr]

I based my implementation on #26. I will wait with creating a PR until it is merged. If you have any feedback already, please let me know.

Am I missing anything else other than the documentation? Is the implementation missing anything?

I am also waiting for MichaelAquilina/flake8-spellcheck#50 to be merged such that the implementation passes the flake8 spellcheck.

Edit: Added docs

[gaborbernat]

@papr now you can open your PR; do remember to add changelog and handle multipath as discussed in #24

[WhyNotHugo]

No, the str variant is the faster. That remains the internal default form.

Well that saves me the trouble of benchmarking it myself. Thanks for testing that.

[gaborbernat]

Creating a class (which pathlib is) will always be slower than just a raw string. See psf/black#1950 for a war story on this.

[WhyNotHugo]

Sure, creating the class itself is slower, but operations on it can be faster due to... implementation.

Anyway, a benchmark is enough, if numbers show it to be slower, there's no point in discussing it further.

[gaborbernat]

I'd be surprised if the core developers have one implementation for os.path and another for pathlib properties. Considering most of os.path is already written in C I don't think I see much opportunity for pathlib to be faster. IMHO pathlib is just a syntactic sugar for the end user (in some cases it's easier) but offers no performance benefits.

[gaborbernat]

Done by #27.