Remove black as a dependency #36
Conversation
Codecov Report
@@ Coverage Diff @@
## master #36 +/- ##
=======================================
Coverage 94.63% 94.63%
=======================================
Files 10 10
Lines 745 745
=======================================
Hits 705 705
Misses 40 40 Continue to review full report at Codecov.
|
| "pytest-xdist~=1.29", | ||
| "tox~=3.14", | ||
| "autopep8~=1.4", | ||
| "black~=20.8b1; python_version >= '3.6'", |
There was a problem hiding this comment.
I assumed that an update to the latest version would not be a problem!
There was a problem hiding this comment.
black formats differently based on its version, so black should probably be pinned in the projects using this library, not here. If it must be here, then probably having it be any version makes more sense?
There was a problem hiding this comment.
@Madoshakalaka thoughts? This issue (#34) is causing problems for us.
There was a problem hiding this comment.
@grizz I've moved the requirement to the dev requirements in this PR, so it won't attempt to install black at all when you use it.
There was a problem hiding this comment.
I'm also running into this issue. Wouldn't upgrading the dev black dependency require reformatting this code base (though I suppose a follow-up PR would be appropriate for that)?
Aside from that, I def like this solution, FWIW.
| else: | ||
| with Popen( | ||
| [sys.executable, "-m", "black", str(file)], stdout=PIPE, stderr=PIPE | ||
| ) as p: | ||
| p.communicate() | ||
|
|
||
| except ImportError: | ||
| # use autopep8 | ||
| import autopep8 | ||
|
|
||
| code = autopep8.fix_code(file.read_text()) | ||
| file.write_text(code) | ||
| try: | ||
| import autopep8 | ||
| else: | ||
| code = autopep8.fix_code(file.read_text()) | ||
| file.write_text(code) | ||
| except ImportError: | ||
| raise RuntimeError("Either black or autopep8 must be installed.") |
There was a problem hiding this comment.
The else before except block is a syntax error; I'd recommend the following:
except ImportError:
# use autopep8
try:
# third party
import autopep8
except ImportError:
raise RuntimeError("Either black or autopep8 must be installed.")
else:
code = autopep8.fix_code(file.read_text())
file.write_text(code)
else:
with Popen(
[sys.executable, "-m", "black", str(file)], stdout=PIPE, stderr=PIPE
) as p:
p.communicate()
|
Solved better in #88 |
4 participants
Fixes #34
Originally this PR just attempted to relax the version pinning, however, that did not work as expected.
Looking at the code in more detail it seems that this library includes support for not having black present in any case, so it's probably better to not have it as a hard dependency.