inspera / blackbricks

Black for Databricks notebooks
MIT License
44 stars 9 forks source link

Add isort for import cells #26

Closed javiruiz closed 1 year ago

javiruiz commented 2 years ago

I've been using blackbricks extensively with my remote Databricks notebooks, and I would like to add isort so it can also reformat cells with imports. Any hints about how to proceed?

bsamseth commented 2 years ago

I've been thinking about this myself, as it would be nice to add. Currently, I make sure to run isort on the notebooks when they are stored locally, and that works fine because the notebooks are just plain Python files in isort's mind. However, this doesn't address running isort on remote notebooks.

The main reason why it isn't already part of blackbricks is that I'm thinking about all other types of linters as well, and adding them one by one doesn't seem like a good idea. Instead I'm thinking about an extension system, so that you can plug in any linter/formatter you want and just let blackbricks handle the orchestration and remote notebook connection. Not 100% sure how to best do that at this time though. Happy to take suggestions.

A temporary solution is to sync the notebook locally, either with git or dbx, running isort, and then syncing it back to Databricks.

javiruiz commented 2 years ago

Thanks for your thorough response! It would be nice to have that extension system. Let's see if we come up with an idea for this. In the meantime, I'll use your suggestion of syncing the notebook locally.

NodeJSmith commented 2 years ago

@bsamseth While writing a different comment I started looking at the flake8 plugin architecture. Is this the kind of system you had in mind? https://flake8.pycqa.org/en/latest/plugin-development/registering-plugins.html

bsamseth commented 2 years ago

@NodeJSmith Yes, it is. I was looking at pytest, but it's using the same entry_points system. It would just define the entry point in pyproject.toml instead of setup.py, but otherwise similar.

The idea would then be that blackbricks would be refactored to only

A few outstanding questions before starting any implementation though:

Happy to hear some thoughts :)

stale[bot] commented 1 year ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.