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

Add pywin32 as optional dependancy #737

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Conversation

nWestie
Copy link

@nWestie nWestie commented Jan 17, 2023

Fix for #735

add pywin32 dependency when installing with the [win] option.
Alternative, it could be added as a platform specific dependency for windows(added, but commented out)

I'm not sure which is the most ideal behavior, thoughts?

@misl6
Copy link
Member

misl6 commented Jan 20, 2023

For now, in order to target issue #735 adding it as an optional dependency is fine (so please remove the additional comment).

In near future (and if you want to, feel free to contribute), I think is better to install platform-specific dependencies by default, in order to be more developer friendly. (Unless the footprint of the platform-specific dependency is huge)

Also, we should consider switching to https://setuptools.pypa.io/en/latest/userguide/declarative_config.html

@nWestie
Copy link
Author

nWestie commented Jan 23, 2023

Great.
I'd agree that it probably makes sense to add platform dependencies automatically, my only question is how that looks when developing for android/iOS; I assume those would stay as optional dependencies, and the dependencies for mac/win/linux would be installed anyways?

As far as updating, would it make more switch to the pyproject.toml standard instead?
https://setuptools.pypa.io/en/latest/userguide/pyproject_config.html
My impression is that it is the most up to date format, so it could make sense to switch to that if there is no specific reason not to

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

Successfully merging this pull request may close these issues.

2 participants