-
Notifications
You must be signed in to change notification settings - Fork 4
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
Add: ruff setup and instructions on autoformatting with vscode #49
Conversation
Is this for the newbie lessons? I usually turn ruff autoformatting off (but leave black on) because i find it extremely annoying to be working on stuff, comment something out to try it, and then when i uncomment it out the import has been removed because something i commented out was the only place that import was being used lol. if it's for the newbie lessons maybe add a caveat like 'you may want to not do this until you get the hang of it' because that could be pretty confusing/frustrating if you don't know what's happening. |
We can disable the autofix for imports: https://github.com/jupyter/jupyter_client/blob/79d315044c69702b71a6985e353b60ddd7921541/pyproject.toml#L253 |
I don't want to derail, but ideally i do want that to be fixable! just once i've arrived at a stable version. for me it's a little bit of a question of 'what is a linter for' - if it's for enforcing code style, then there isn't really a strong need to have it run on every save. if it's for formatting, then we are in the odd territory of ruff being a linter and a formatter, where typically you'd expect a formatter to not change anything about the semantics of the code and only make cosmetic changes, but people often use ruff in a sorta hybrid way where they make formatting changes that change the structure of the code. anyway it's only rly a concern if this is intended to be part of the newbie lessons in the workshop next week, and it's only beause i've been like viscerally frustrated by this and i can imagine that being a problem we don't want to introduce for someone in a time-limited workshop, otherwise ignore me, totally fine with having this in the pyos package aside from that. |
Hi Friends
to answer your question @sneakers-the-rat the idea of this repo specifically is that it's eventually going to be a fully fleshed out package. Yes this means that it might be more complex than we want for a beginner workshop. But the idea here was to add docs around things like setting up ruff, black, pre-commit and then having a real example for people to see what a pacakge can look like. For our workshop next week i was thinking we could use this repo for the third workshop -sharing code these lessons are about connecting code to zenodo, adding a doi badge, making a release, publishing to (test) pypi and conda forge. This repo has a codespace setup wtih vscode to support that but i need to add hatch to it! I'd like to have documentation about ruff in our docs for this repo. People can chose to remove it and not use it and we can warn them about issues too! If we wanted however, we could also create a much simpler package for them to fork and rename during the workshop too. i'm assuming that
Does that help clarify? so i do want to get this pr merged. But i'm open if we decide that we want a MUCH simpler package to use next week for the workshop. open to any and all thoughts!! |
Gosh @sneakers-the-rat one more thing - i use black locally as an autoformatter too! I actually havn't tried ruff yet in vscode. I do use ruff in some packaging work however and so far i like it. i believe that we could setup a codespace with black in it (i just need to figure out how to do that!) |
Great! works for me. |
I've been frustrated by ruff running on file save, too, as it likes to remove "unused" imports during some refactoring I do. Ruff does have the |
Ok y'all. i hear you! I have just updated this pr with new edits that were taken from #34, which I closed by mistake while working on lessons 😆 now that I've learned a hard lesson about why to merge PRs more quickly - let's go ahead and merge this as is. BUT please review the docs and we can back off of ruff as a development tool on save and stick to pre-commit and CI for those types of checks (if that sounds good to everyone). Let's open issues and iteratively update the documentation for this package as it makes sense. for our workshop, this is just a demo for people to look at with the goal of eventually fleshing it out with ci and other features that we may never get to in a beginner-friendly workshop, but others who use our template and know more about packaging may appreciate it! |
pre-commit.ci autofix |
@all-contributors please add @blink1073 for code, review |
I've put up a pull request to add @blink1073! 🎉 |
TODO: pull all comments from here into this pr and merge it referencing the pr i accidentally closed.