Support platformdirs as drop-in replacement for appdirs#1598
Conversation
Test Results321 tests 320 ✅ 1m 43s ⏱️ Results for commit 2360ca4. ♻️ This comment has been updated with latest results. |
|
The alternate features thing is interesting, I like how it's an easy automatic substitution. But thinking ahead towards the eventual removal of appdirs, I'm not sure that the extra code is necessary, and it might cause confusion for users. With the proposed change, when Adding platformdirs as a whole separate feature and importing like this might be a little simpler: if features['platformdirs']['present']:
import platformdirs as appdirs
elif features['appdirs']['present']:
import appdirsIt's just a design/opinion thing. @andreleblanc11 or @petersilva do you have any preference? Hopefully I don't get hit with the wrath of AI for this :D |
|
So I reworked this to use your approach - platformdirs is its own feature entry now instead of the Alternate_modules thing. That way |
|
I don't see any new code on this branch, did you use a different branch or maybe forgot to push the change? |
|
Woops, I had it stashed locally and forgot to push — just pushed the fix. platformdirs and appdirs are now separate feature entries instead of using the Alternate_modules fallback. Should be good now! |
e8a5d25 to
2360ca4
Compare
|
Pushed a refresh for this branch. This is now rebased onto current @reidsunderland the code that was missing before is definitely in this branch now:
I also added focused tests so this is explicit:
Tests:
|
|
This looks good, thanks for the updates. I should have noted this before, but the packaging stuff should also be updated to prefer platformdirs where available.
There's a lot of places to adjust and I'm not completely sure how to set up alternatives for each. In Debian we can use: But I'm not sure about the others. If you want we could merge this PR and deal with that separately? |
Summary
Alternate_modules— if the primary module isn't found, alternates are tried before disabling the feature. Theappdirsfeature listsplatformdirsas an alternate.platformdirsis preferred overappdirssince appdirs is unmaintained and unavailable on RHEL9. Both provide the same API for the three functions we use (site_config_dir,user_config_dir,user_cache_dir).Closes #1493
Test plan
sr3 featuresshows appdirs as present when onlyplatformdirsis installedsr3 featuresshows appdirs as present when onlyappdirsis installed