22 ms·
Show HN: Refurb – A tool for refurbishing and modernizing Python codebases
- c54 4y agoI'm just getting into the python ecosystem in a professional capacity. How does refurb compare to the landscape of other python linters and static analyzers? Is it meant to be used in concert with others or to be more of a one stop shop? It looks like the checks are in the category of standardizing on modern python syntax features, sort-of one step up from an autoformatter like yapf or black. Is this correct?
- dosisod 4y agoYes, I would say that is correct! Like I mention at the very bottom of the README, Refurb is not meant to be an all-in-one linter or formatter: Tools like black, flake8, mypy, etc. already exist, so trying to recreate what they do would be a lot of work. Refurb is more of an addition on top of the tools I previously mentioned, and less of an altogether replacement.
- thumbcore 4y agoRelated -- I'm curious how you'd compare this to pyupgrade https://pypi.org/project/pyupgrade/ https://pypi.org/project/pyupgrade/
- dosisod 4y agoInteresting, I have never heard of this project before! From looking at the README, Refurb is more focused on simplifying the codebases, making them more expressive, whereas pyupgrade seems to have the goal of upgrading you from version A to version B (specifically Python 2 to 3). Near the bottom there are some newer Python version (3.6+) related upgrades, which I was considering adding to Refurb. I expect there to be some overlap between my project and some of the existing projects out there, so I will have to weight the costs of adding these in vs leaving them out. Thanks for mentioning that project!
- justusw 4y agoI understand that some of these suggestions have been made possible by recent changes to Python 3. But how do I know whether a specific change suggestion will genuinely improve my codebase vs. just confuse Python developers who are used to seeing things implemented a certain way (e.g., Path lib vs open() )? It would certainly make sense to add more information on the tradeoffs and what long term benefits writing it in a certain way will be.
- dosisod 4y agoI agree, better clarification/classification of errors would be nice, as well as the different tradeoffs you should expect from using a particular check. In particular with the Path() related checks, they can seem contrived when used in isolation (open() is a pretty common idiom, like you said), but they become really powerful when you chain them together: (Path("some/deep/folder").parent / "file.txt").read_text() Similar to black, Refurb is opinionated. There are probably going to be checks which you will ignore. Somewhat of a non-goal with Refurb is bringing to light some of the different ways of writing Python code, and the pathlib module is, IMHO, a very underutilized part of the stdlib.
- justusw 4y agoI agree, pathlib is really powerful, and I can see myself using it for new developments. May I then assume that Refurb is better suited to work on new code? To get back to open(), I don’t know if I would want to go through my company’s legacy code (which runs on Python 3.10) and start replacing battle-tested code everywhere. Same would go for tuple literals over lists in certain circumstances. I think this will go really well with choosing an „acceptable“ subset of Python for your project, similar to how some companies choose a subset of C++ and then stick with that throughout a project.
- dosisod 4y agoI think that using Refurb on new projects would be a great use case, but even for existing/old/legacy projects, Refurb can still be a good option, it really just depends on what is already there. I agree that going and updating your company's codebase would probably be a bad idea, considering that there are some kinks here and there. Refurb tries its best, but it is very early on in its development, so errors that are emitted should be taken as suggestions, not gospel.
- simonw 4y agoThis is the largest codebase I've seen that makes extensive use of the new Python match: statement introduced in Python 3.10 - example: https://github.com/dosisod/refurb/blob/master/refurb/checks/pathlib/read_text.py https://github.com/dosisod/refurb/blob/master/refurb/checks/...
- datalopers 4y agoThat code, starting on line 41, has convinced me to continue avoiding Python. Python has went from a clear concise language with one right way to do things, to outpacing C++ when it comes to adding esoteric language features.
- zem 4y agothat code is pattern matching on an ast node; the complexity is from the data structure involved, not the language. there is no more concise way to match a specific node and get at the relevant fields, just various different forms of verbose code.
- ac130kz 4y agoThat code has nothing to do with conciseness, he could've extracted the arguments in separate variables
- jraph 4y agoIf anything, this matching on an AST is very readable and elegant, and that just convinced me that the syntax of Python seems really good to write a parser.
- sireat 4y agothat AST parsing code looks like something written in Scala ... seems like Scala is moving to more Python like and Python is moving to add some features from Scala.
- Waterluvian 4y agoI’m actually genuinely curious what alternative you might present from any language. I always thought that was relatively concise for such a complex data structure.
- asplake 4y agoTo the example, why recommend replacing a list with a tuple when it is about to be iterated through? I’d choose a tuple if it was to be destructured, but not for this. I know it works either way, but this one feels off.
- physicsguy 4y agoSimilarly, some of these are just style. Using Path(x).read_text() rather than a context manager for opening the file for e.g.
- pantsforbirds 4y agoLists can be mutated and Tuples can't, so when you want a default iterable parameter in a function, for example, you would want to use a Tuple. I assume this particular rule is about trying to stay consistent to that practice, even if you only access the iterator of the list and not the list itself.
- dosisod 4y agoBecause tuples cannot change over time, they are (slightly) faster to create compared to lists. Like you mentioned, they are being created just to be iterated over. It is a small enough of a performance improvement that it more of a style choice then anything. Bonus fact: You can use set iteration in for loops as well! This has the added benefit of sorting the values as well: >>> for x in {2, 1, 3}: ... print(x) 1 2 3 You didn't ask for that, but I felt like sharing it, to there you go
- falcor84 4y agoHere's another tip of syntactic trivia - you can leave the brackets out entirely to (arguably) make it clearer that your don't care about the data structure, and it will implicitly use a tuple: >>> for x in 1, 2, 3: ...
- 5d8767c68926 4y ago>... This has the added benefit of sorting the values as well I don't think you can rely on this? Happy to be proven wrong, but set does not guarantee iteration order. There may be a distinction if all elements are available at set construction, but that seems like a fiddly rule I would rather avoid.
- ogarten 4y agoHave to check it out. I wish there was a reasoning behind the suggestions though. Why choose a tuple instead of a list? I won't learn much from just following along. I am also much more likely to change something when I know the reason. Will follow the project.
- magarnicle 4y agoIt has an --explain flag for this purpose.
- dosisod 4y agoYes, there is a --explain flag for explaining the checks in more detail. I thought about adding a "use --explain ERR for more info" at the bottom if there is 1 or more errors, probably would be best to add this in.
- blondin 4y agowhy replace open() with the Path().read_text()? the open() API transcends Python and is almost ubiquitous. that suggestion seems frivolous. while we are talking modern Python, the type hints fever seems to be receding. such a good thing in my opinion. i expected this tool to bark about types but am glad the author didn't go that far. nice tool.
- dosisod 4y agoSomething that I forgot to mention in the other answers similar to this is that the Path().read_text() check only applies if the open() block is a single line, and that line is only used to read the contents of the file. If you are doing other things in that block, no error will be emitted. I agree that open() is pretty ubiquitous, but in certain cases (ie, if I am already using a Path object), using read_text() is a nice one-liner.
- bityard 4y agoread_text() is simpler and _much_ clearer (and thus, Pythonic) when you're doing trivial file operations like reading the contents into a variable. Opening up a whole context manager is still an option when you have to do more complicated stuff to the file.
- awestroke 4y agoFrom reading these comments, I get the impression that python developers don't really want to modernize their codebases. This fits with my experience of python developers resisting new language features like types and even python v 3.
- desindol 4y agoMost early unicorns were or are still python monoliths…
- awestroke 4y agoOh, really? Which ones?
- LittlePeter 4y agoNot sure why you are down-voted... Instagram and Dropbox, both used Python.
- IshKebab 4y agoDropbox is the only one I know of but as far as I know they have started migrating to Go. I guess they'll never fully migrate because they have a lot of code.
- dagurp 4y agoAs the Zen of Python[1] says: "There should be one-- and preferably only one --obvious way to do it." so this is on brand :) 1. https://peps.python.org/pep-0020/ https://peps.python.org/pep-0020/
- carapace 4y agoI'm an ex-Python developer. (I didn't survive 2 to 3.) What I see happening is Python getting "improved to death". As people keep adding "features", aka complexity, to Python it's moving away from its niche and coming into more direct competition with other languages and ecosystems (e.g. Rust, Go, Nim, Haskell, OCaml, even Ada and Java!) From my POV this tool, though technically very neat, does unnecessary work to make things harder to understand. Try to see it this way: all this Red Queen's Race, this running hard to stay in the same place, the endless "improving" of languages like Python and JS, are an attack on your knowledge and skills: just sitting there you are becoming obsolete not because your knowledge is degrading but because the youngsters keep changing the tune forcing you to learn new dances just to stay relevant. There is very little new under the sun in IT: things like type checking and inference are decades old.
- Dunedan 4y agoAs usual be attentive and suspicious when using such tools as its suggestions might not be appropriate. For example let's take the following code: with open("foo") as a: contents = a.read(100) Running it through refurb produces the following suggestion: > main.py:1:1 [FURB101]: Use `y = Path(x).read_text()` instead of `with open(x, ...) as f: y = f.read()`
- dosisod 4y agoYes, Refurb error messages are not gospel: Make sure the changes make sense! Thank you for bring this bug up, I will make sure to address it tomorrow.
- dosisod 4y agoAlso, I don't know if this was part of your critique, but are you also mentioning that the "a" variable is not in the suggestion? The reason behind that is long names, such as "super_special_file" would greatly increase the length of the error messages, making it hard to see the "meat" of what is being changed. Hence the x and y naming. Perhaps I should add a flag to allow verbatim (or close to it) errors. Recreating the source lines exactly will be pretty hard, but I think we can do better here.
- sfink 4y agoPerhaps substitute the actual variables if they are simple and short, otherwise do something like: > main.py:1:1 [FURB101]: Use `y = pathlib.Path(x).read_text()` instead of `with open(x, ...) as f: y = f.read()`, where y=my_important_data and x=super_special_file Btw: thanks, your tool suggested a handful of things I didn't know about, and pushed me towards using pathlib instead of relying on my muscle memory to do the laborious `os.path.join(..., ...)`. One request I would have is to spell out the full name of suggested imports. I had never heard of `contextlib.suppress`. And in the above message, I think it will be obvious that you could do `from pathlib import Path` instead of the literal text in the suggestion.
- dosisod 4y ago
- purrcat259 4y agoHaving gone recently through a push at $WORK to modernize our 3.7 codebase up to 3.9, I'd love to try using this tool. Does it really only run one file at a time? I'd love for a recursive folder option.
- dosisod 4y agoRefurb can take any number of files or folders, for example: refurb file.py folder/ another_folder/ Hope that helps!
- ac130kz 4y agoWhy not a Pylint plugin though?
- claytonjy 4y agoI don't know what it takes to make a pylint plugin, but maybe it's cleaner to have a standalone `refurb` and then a `pylint-refurb` could be very simple?
- ac130kz 4y agoFrom the pylint documentation it seems the opposite, you just have to extend some methods, and that's it
- dosisod 4y agoThe reason I didn't just create a pylint/flake8/mypy plugin is because they all use a very specific methodology, whereas I wanted the freedom to architect it the way I wanted. Also, I really needed good type information, which is why I used mypy[0] as the AST parser/type checker. [0]: https://github.com/python/mypy https://github.com/python/mypy
- elanning 4y agoThis looks like a nice piece of work. I hope the author continues on it, or other fun hobby projects.
- brhsagain 4y agoI like how it teaches me things about Python that I didn't know. Just in the example, I didn't know about Path.read_text or str.startswith(<tuple>).
- bityard 4y agoI didn't know about read_text() either and I was even somewhat embarrassed when I looked it up in the docs and discovered that it (and its bytes/write counterparts) were added in 3.5 which is basically forever ago.
- usrme 4y agoAre the authors aware of Sourcery[^1]? I've been using it for a long time to clean up and modernize my Python codebases, and am wondering how Refurb can either supplant Sourcery or augment my usage of it? --- [^1]: https://sourcery.ai/ https://sourcery.ai/
- mdaniel 4y agoI got a good chuckle out of them having a PyCharm plugin. I'm not going to create an account just to kick the tires on that, but I'd be stunned if they out-magick PyCharm
- dosisod 4y agoI have never heard of Sourcery, but I will take a look at it. From what I can tell, Refurb is a CLI only tool (at least, for now), whereas Sourcery is more aimed at IDEs. Refurb should play along nicely with most other linters, though you may need to configure things first.
- bityard 4y agoSourcery looks to be a commercial product of some kind whereas I can just `pipx install refurb`
- nicoco 4y agoNice! Passing it on a small project I work on I learnt 2 things already: `Path.read_text()` and `with suppress(SomeException): ...`.
- deleted 4y ago[deleted]
- deleted 4y ago[deleted]
- svilen_dobrev 4y agoheh ...$ ~/.local/bin/refurb -h refurb: unsupported option "-h" ...$ ~/.local/bin/refurb --help refurb: unsupported option "--help" ...$ ~/.local/bin/refurb refurb: no arguments passed ah. have fun