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
[MRG] put black dependency as optional #120
[MRG] put black dependency as optional #120
Conversation
setup.py
Outdated
@@ -71,6 +71,7 @@ | |||
'inverse_covariance.profiling', | |||
'inverse_covariance.pyquic'], | |||
install_requires=INSTALL_REQUIRES, | |||
extras_require=dict(contribute=['black==18.6b4']), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe instead of "contribute" something like "style" ?
I checked and with this modification the install works with python2.7 |
I think the right thing to do here is split out the dev requirements into a separate file as I've done here: https://github.com/jasonlaska/spherecluster |
That's nice, something like this ? |
.travis.yml
Outdated
@@ -27,6 +27,7 @@ install: | |||
|
|||
# command to run tests | |||
script: | |||
- pip install black |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of this, change line 25 to install the dev-requirements
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(the dev requirements are a superset of the requirements so you should only need to reference the one file)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Side note: since this version of black isn't tied to the version in dev-requirements, the tests fail (Black makes minor breaking changes to formatting in each version).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah yes, indeed, thanks !
Minor comment about the travis script, but yes, this is good. |
LGTM, I think the tests are breaking due to a non-deterministic bug. |
Thank you @wdevazelhes for your contribution. |
Is it possible to deploy a release to PyPI to include this update? |
Fixes #119
I couldn't install
skggm
with python2.7 because black does not seem to exist in python 2.7 (see https://github.com/ambv/black#installation). I thought this could be a fix (puttingblack
as an optional dependency), but I may be totally wrong here