-
Notifications
You must be signed in to change notification settings - Fork 9
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
Release candidate 1.0 #25
Conversation
BREAKING CHANGE: Obviously. No longer required due to Databricks adding option for 4 space indentation. So in keeping with Black's philosophy, we don't keep options that are not needed.
Bumping everything and re-testing. Most importantly, bumping black.
Parts of the code based on contribution in #20 Co-authored-by: Niels Lemmens <1936122+bulv1ne@users.noreply.github.com>
Although it disagrees with black, Databricks does their notebook formatting their way. To avoid perpetual changes, blackbricks is forced to replicate their oddities.
f6d458c
to
eb655a2
Compare
@bsamseth I have a concern about dropping support for Python 3.8. The Databricks 10.4 runtime is their latest LTS, and it uses 3.8.10. Dropping support for 3.8 would make it much harder to use this package in an environment set up to develop for the 10.4 runtime. Is there a large need to remove support for 3.8? |
@NodeJSmith That's a fair point. I've never actually used blackbricks from a Databricks cluster, so I didn't even consider this. I have it installed as a binary locally, and never felt the need to have it elsewhere. The main reason for dropping support for 3.8 is maintenance. I was concerned about accidentally using a 3.9+ feature without thinking about it, and not noticing because I don't use 3.8 on my machine anymore. I figured it was better to explicitly not support it. However, my guess is that you are not alone in this. I'm inclined to keep 3.8 support because of it, at least until the next LTS has been out for a while. Out of curiosity, what is is the benefit of having it installed on the cluster in your mind? Blackbricks is an application, not a library, so the dependency requirements are intentionally made rather strict to ensure it works correctly. Assuming you install it through the libraries tab in the cluster config, that would likely cause dependency conflicts whenever you install a library that shares some dependencies with blackbricks. Perhaps this simply hasn't come up as an issue in your case? |
@bsamseth We are not using blackbricks on the cluster itself, but we create virtual environments for development with the same Python runtime as our job clusters use. This means that we always run Python 3.8.10 locally, which is where we would be installing and running blackbricks from. We won't be able to use blackbricks the way we currently are, if support for 3.8 is dropped. |
Ref #25 (comment) Want to keep support for 3.8 at least until Databricks has a LTS runtime with 3.9 available.
@NodeJSmith That makes more sense, thanks. I would still worry that you could get dependency conflicts with this approach, but if that isn't an issue then I see the benefit of bundling up everything for the sake of consistency. I've restored 3.8 compatibility in fc2eed8. |
Candidate for a 1.0 release. The core formatting rules haven't changed for a good while, and seem to work well against Databricks' notebook format. I also want to remove the black-patching to use two spaces for indentation, now that Databricks is no longer forcing 2 spaces. Doing so will be a breaking change, which is a good time to bump the major version.
Biggest changes:
Drop support for Python 3.8 and below. It's been long enough since 3.9 was releasedVersion 0.7.0 deprecated two-space indentation on 2022-09-03. In order to give users a chance to notice the deprecation warning, version 1.0 is scheduled for release on 2022-09-11. That's not a big window, but both from the 0.x version and the beta tag on PyPI there was never a guarantee of stability.