Use platformdirs over appdirs #2375
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
cooperlees
approved these changes
Jul 16, 2021
There was a problem hiding this comment.
Sounds like a smart move to me. Thanks @gaborbernat !
- Can we please just make a note in
CHANGES.mdabout the switch just to inform people
|
CLI will be faster too (every bit helps):
|
JelleZijlstra
approved these changes
Jul 16, 2021
There was a problem hiding this comment.
Thanks, looks good!
@ofek just curious, what changed in the fork to make import time faster?
|
@JelleZijlstra Number 1. of tox-dev/platformdirs#1 + lazy computation of things |
Signed-off-by: Bernát Gábor <[email protected]> Signed-off-by: Bernát Gábor <[email protected]> Signed-off-by: Bernát Gábor <[email protected]>
ichard26
approved these changes
Jul 16, 2021
There was a problem hiding this comment.
Oh nice, one less unmaintained dependency, and that import time reduction! 🎉 I hope the pipenv locking wasn't too painful :)
Thank you for the PR!
By the way, if time allows it would be great if you could add any comments on your contributing experience here: #2238. I understand that you're probably busy though. P.S. thanks for maintaining virtualenv 😃
9 tasks
4 tasks
This was referenced Aug 31, 2021
ichard26
pushed a commit
that referenced
this pull request
Sep 1, 2021
Add new platformdirs dependencies as hidden imports when creating PyInstaller-based binaries. platformdirs imports the module for each platform dynamically, which PyInstaller is unable to correctly detect for packing. By adding the modules as hidden imports, we are telling PyInstaller to include the modules in the packaged binary. This issue seems to have been introduce when switching to platformdirs in #2375. fixes #2464 Commit history before merge: * Add hidden import to PyInstaller build Add new platformdirs dependency as a hidden import when creating PyInstaller based binaries. * Only include the platformdirs for the relevant os
12 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
As the appdirs repositories, maintainership has dried up (ActiveState/appdirs#79) we've forked the repository under platformdirs (https://github.com/platformdirs/platformdirs) name.
We've merged some existing open PRs and adding Android support (tox-dev/platformdirs#15) is currently in the works. We encourage black to switch to it.