Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Delete toposort dependency #1507

Merged
merged 12 commits into from
Jun 2, 2024
Merged

Delete toposort dependency #1507

merged 12 commits into from
Jun 2, 2024

Conversation

Akuli
Copy link
Owner

@Akuli Akuli commented Jun 2, 2024

Given that xz just happened, I want to minimize Porcupine's dependencies, and I think toposort is the only dependency that can be reasonably removed.

When I added toposort dependency in 2017 (commit c713798), it was because I couldn't figure out how to program it myself. The source code is short, but it looked complicated and I didn't want to copy it. Since then, I have worked with graphs more and the algorithm is now quite obvious to me, so I rewrote it from scratch.

porcupine/pluginloader.py Outdated Show resolved Hide resolved
@Akuli Akuli merged commit 9be5765 into main Jun 2, 2024
18 checks passed
@Akuli Akuli deleted the bye-toposort branch June 2, 2024 15:52
@Moosems
Copy link
Contributor

Moosems commented Jun 3, 2024

Why can't platformdirs be removed?

@Akuli
Copy link
Owner Author

Akuli commented Jun 3, 2024

Good question. It can be removed, but it would require a few hours of Windows hell.

Porcupine only uses user_log_path, user_cache_path and user_config_path. Determining them on 3 different platforms gives 9 different cases. It would be slightly more work than this PR, but maybe worth the effort.

As for the windows hell: In platformdirs source code, they have 3 ways to locate folders (ctypes, registry, environment variables). They dynamically pick whatever works when platformdirs is imported. Environment variables are by far the simplest way, and probably reliable enough for Porcupine, but this needs to be tested in a few different Windows environments to convince me.

@Moosems
Copy link
Contributor

Moosems commented Jun 3, 2024

There's a reason I've decided to cut windows support in all my future projects lol

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants