psf / black

The uncompromising Python code formatter
https://black.readthedocs.io/en/stable/
MIT License
38.66k stars 2.43k forks source link

Simple CLI "client" For blackd #2405

Open the-wondersmith opened 3 years ago

the-wondersmith commented 3 years ago

According to the black docs there's no "official" client for formatting files with blackd. I've been having trouble with the available plugin for PyCharm (BlackConnect) randomly throwing strange errors on files with more than ~800-ish lines.

Anyway, I whipped up a simple cli script to fill that gap. It has no dependencies outside of the standard library, and should run just fine in Python 3.5+ (personally tested against Python 3.7, 3.8, 3.9).

It'd probably be prudent to add support for the rest of the (admittedly few) arguments blackd takes, but in fairness I did design it as a temporary replacement for Black Connect in PyCharm so simplicity was at the forefront. Anyway, if there's any interest in it, I'd be more than happy to update it to support the gammut of blackd options.

HassanAbouelela commented 3 years ago

I can’t make a comment about whether or not this will be accepted, but upon a quick look at the linked script, it should probably use click like the main black module.

cooperlees commented 3 years ago

This sounds interesting and useful. I agree, lets make it click and support all the options. I am for it and if other maintainer's are I'm sure we'd accept it. Speak up other maintainers if you're against it.

Please make it another entrypoint in the setup.py like blackd itself:

And place the source file in the src/blackd directory.

the-wondersmith commented 3 years ago

@cooperlees Can do. I'll submit a PR at some point this week.

ichard26 commented 3 years ago

I like this idea too :)

FWIW, I'm starting to think we are shipping too many entrypoints and it may be better to add this functionality under the blackd name. An example would be the dmypy entrypoint shipped with mypy, it has both run and start subcommands. Now dmypy works as a daemon so it works differently from blackd, but I don't think that's a big deal. I feel like having "blackd to start a server" and blackd-cli as the client" would be a bit confusing. Just my 2cents.

JelleZijlstra commented 3 years ago

I'm curious what the performance characteristics of this script are. The main point of blackd is to avoid the cost of Python startup. This client script is also in Python, so we'd still pay that cost. Then again, it doesn't import as many packages as black itself does, so maybe it's still a win.

ichard26 commented 3 years ago

Hmm, yeah maybe my idea to combine the entrypoints might not be the best idea since startup performance is the number one priority here.

I'm curious what the performance characteristics of this script are.

Well I happen to be in "benchmarking mode" today so here's the import time characteristics (ignoring Python's own startup cost):

+-----------+--------+-----------------------+-----------------------+
| Benchmark | black  | blackd-client         | blackd-client+click   |
+===========+========+=======================+=======================+
| timeit    | 231 ms | 58.5 ms: 3.95x faster | 84.6 ms: 2.73x faster |
+-----------+--------+-----------------------+-----------------------+
JelleZijlstra commented 3 years ago

Thanks, that suggests this does really help speed—getting your code autoformatted 150 ms faster should be noticeable.

the-wondersmith commented 3 years ago

Apologies for the delay on this.

I was able to "get back to" the project, and ended up re-creating the client in Rust... whoops. In my defence I wanted it to be a bit more performant than a native Python script would be because I personally intend to use it as the on-save auto-formatter for my IDE.

I'm newer to Rust than I am to Python, so I can't in good conscience say that the code is good but I will say that I'm personally using it and I haven't had any issues out of it. If there's still an interest in incorporating it into the black project itself (which I'm all for), I'll happily "convert" it to PyO3 so that it can be used as a native Python extension.

In any case, I've published the Rust code and a MacOS binary here. If anyone wants binaries for other platforms just leave a comment and I'll do my best to run the build accordingly.

This is the "help", if anyone is curious:

blackd_client
the-wondersmith commented 3 years ago

Well... with more than 7 days since I posted the Rust client and no updates or seeming interest, I'm going to go ahead and close this issue.

If there is any interest though, please open it back up or let me know via the Rust repo. It would be really cool to have blackd ship with its own equally fast client.

ichard26 commented 3 years ago

Ah sorry @the-wondersmith for taking so long to respond! We're definitely interested in or at least open to introducing native code to speed up Black. I'm actually working on integrating the mypyc Python-to-C-extension transcompiler with the core black codebase: #2431. The main difference is that the C is generated on the fly during the CD pipeline while this introduces native code within the repository. Also it's in Rust which so that's another curve-ball, but all of this is worth exploring!

It would be really cool to have blackd ship with its own equally fast client.

100% agree, thank you so much for your work on this!


I'll try to look into this further soon, but my initial thoughts are that we might have to break this out into a separate repository / package unless we can find some sane not-scary way of packaging a Python, C, and Rust distribution with setuptools :p

the-wondersmith commented 3 years ago

@ichard26 I guess now I have to apologize for the delay in responding, whoops. 😅

My hunch is that a separate repo might be best, if my Rust implementation wins out. I've made some improvements recently, to minimize the binary size and colorize the terminal output.

The reasons I'd advocate for the rust implementation are:

As I said, I'd love to work out details if the black team is interested. If so, just pop me an email (the at wondersmith dot dev) and we'll get it all squared away. 😁

HassanAbouelela commented 3 years ago

I'm not really following what's happening here (I just see it pop into my notifications from time to time), but I'll put in my two cents.

There is a certain maintenance burden with every new feature to a project, but that burden is especially heavy when you don't have people that can actively carry it. Safe to say, most people that end up here have experience with python, but it's hard to say if the same is true of Rust.

The vim plugin is (was?) a good example of this. No one who knows/wants to maintain it, and more and more bugs popping up. At one point the discussion about it drifted to making a decision on whether to try and support it with minimal contributors, or drop it and break workflows for existing users.

If you're thinking about implementing this (in black, not a standalone project), is there really no alternative that's less burdensome?

the-wondersmith commented 3 years ago

@HassanAbouelela I completely understand the burden of maintenance, and I don't disagree at all. In this specific instance, the rub is that the whole point of blackd is to avoid the startup overhead of calling black from outside a running Python instance. As @ichard26's testing shows, that's not insignificant.

The ultra-lightweight Python client I wrote apparently didn't satisfy because (while light and fast) it doesn't use click like the rest of the black codebase does. I re-wrote the client in Rust as a personal thing because 1) I've been learning Rust specifically for use in writing high-speed Python extensions and 2) because I wanted the absolute fastest, most reliable client with the absolute lowest maintenance burden I could possibly produce and I think Rust fits that bill nicely.

By way of compromise though, I think I can set up local cross-compilation of binaries for Windows and Linux. Assuming that's the case, I'll throw together a project that packages a dummy Python module along with the compiled binaries into platform-specific Python wheels and publish it to PyPI. That will allow users to install it seperate from black and for black to list the client as an optional extra and remove the burden of maintaining the Rust code.

Otherwise, I think the only thing that would make sense (i.e. preserve the low-overhead requirement) would be to update the original click-less client I wrote to support the other blackd argument flags I didn't initially write in and include that in the black codebase itself. Anything else I can think of would either incur extra runtime overhead (thereby defeating the point of blackd) or require separate or semi-separate maintenance in the black codebase (thereby falling afoul of your maintenance concern).

Thoughts?

taleinat commented 1 year ago

FWIW my two cents as an outsider who would like to use blackd + a fast client, especially for pre-commit hooks:

ichard26 commented 1 year ago

For pre-commit hooks, the interpreted version of Black is still used. Eventually I'd like to see it use the compiled version of Black too. For more information see #3405.

Yeah.. not sure I was thinking earlier. I'd be happy to accept a simple Python blackd-client or whatever[^1]. While we prefer Click, it might be faster to just stick with argparse and avoid importing too many packages. Some benchmarking would be required here.

[^1]: Annoyingly enough, the blackd CLI wasn't designed with extensibility in mind. Unlike dmypy, it's not easy to add a subcommand for interacting with the HTTP server.