from Hacker News

We fixed f-string typos in popular Python repos

by rikatee on 4/29/22, 6:06 PM with 145 comments

• by mhils on 4/29/22, 8:32 PM

  We've been one of 666 repos, and I'm not too happy of having our repo used as advertising space. Some thoughts:

  - I'm happy to receive fix-a-typo PRs from human users. In this case the other side demonstrated that they care by putting in a bit of manual effort, and a small PR often paves the way towards larger contributions. I also know that open source beginners get really excited about their first small contributions, and I'm honestly happy to support that.

  - In contrast, the marginal effort for bot PRs is ~0. It's very easy to generate a small amount of work for a lot of people, and the nice side effect is that the bot's platform is advertised everywhere. As a maintainer, I have never given consent to this and I have no choice to opt out.

  We are very happy users of some GitHub bots, but I feel it needs to be an active adoption decision by the maintainer. If you want to pitch me your service you may send me an unsolicited email, but don't use our public space to advertise your product without asking.

  Edit: I don't want to be too harsh to OP here - at least they pointed out a small but valid issue in our case. I very much appreciate their apology at https://news.ycombinator.com/item?id=31210245

• by HL33tibCe7 on 4/29/22, 6:46 PM

  > > We may be looking too deep into this but it seems like many developers think when string concatenation occurs it’s enough to declare the first string as an f-string and the other strings are turned into f-strings by osmosis. It doesn’t. We’re not suggesting this is the case for all developers that accidentally did this error, but interesting nonetheless.

  I highly doubt that people believed that f-strings worked this way. Far more likely is that, for example, the expression started as one line, then got split onto two, or some such similar scenario.

• by gus_massa on 4/30/22, 1:20 AM

  I don't mind it's a bot, and I really appreciate that apparently a human made the final review before sending the PR. But I don't like that the tittle of the commit is:

  > Fix issue probably-meant-fstring found at https://codereview.doctor

  I expect a more neutral title for a commit, something like

  > Fix fstring in <name-of-file>

  Each maintainer/project has their own (weird) rules about titles, and if any other files must log the changes, and regression test, and whatever they like. But I think no maintainer/project expect to see the name of the author in the commit tile.

• by bvinc on 4/29/22, 6:38 PM

  So they checked 666 python repositories and fixed bugs in 69 of them. Interesting choice of numbers.

• by memco on 4/29/22, 6:54 PM

  The article links to some docs for the logging module here: https://docs.python.org/3/howto/logging.html#optimization asserting that f-strings are less optimal but the docs do not say that they do not optimize our the expression evaluation of f-strings: only that the logging module tried to perform evaluation as late as possible: where is the f-string described as suboptimal?

  Relatedly the logging optimization suggests setting: raiseExceptions to false for production logging code: where is that set? On the logger, handler or something else?

• by readthenotes1 on 4/29/22, 6:40 PM

  I find it ironic that the article points out that relying on error from humans to find errors is something of a hit or miss proposition and suggests that automating error finding is an appropriate course instead of making it less likely to make the error in the first place.

  For example, I wonder how many errors would have been found if the definition of a format string was the default? That is, how many times would people have written something like "hello {previously-defined-variable}" and not meant to substitute the value of that previously defined variable at runtime?

• by dewey on 4/29/22, 6:32 PM

  > For science you can see the reactions here.

  That link seems to be broken: https://github.com/issues?q=is%3Aissue+author%3Acode-review-...

  I was actually surprised to read that people would ignore or be annoyed by a bot raising a valid PR that can be easily merged after a quick glance. What would be the reason for that?

• by mjs7231 on 4/29/22, 9:43 PM

  This was posted on Reddit earlier this week with similar negative responses: https://www.reddit.com/r/Python/comments/ubkvrd/10_of_the_66...

• by f7fg_u-_h on 4/29/22, 7:53 PM

  > After creating 69 pull requests the reaction ranged from:

  > Annoyance that a bot with no context on their codebase was raising pull requests. A few accepted the bugs were simple enough for a bot to fix and merged the pull request, but a few closed the pull requests and issues without fixing. Fair enough. Open source developers are busy people and are not being paid to interact with what they think it just a bot. We’re not entitled to their limited time.

  > Neutral silence. They just merged the PR.

  > Gratitude. “thanks” or “good bot”.

  I appreciate their self awareness about responses from maintainers.

• by malcolmgreaves on 4/29/22, 7:19 PM

  You can also use flake8 to find this, and even more, errors in Python code.

• by fareesh on 4/29/22, 7:04 PM

  I like python although I don't use it too often. Would it be unfairly critical of me to say that this is the outcome of a bad design choice? Ideally languages should be designed in a way that a bug like this which is so widespread and easy to create, should be caught via some mechanism, either linting or some part of the process.

• by pabs3 on 4/30/22, 2:25 AM

  Its a shame Code Review Doctor isn't open source, then everyone could install it and use it on any code they write.

• by zikohh on 4/29/22, 9:11 PM

  Can I use code review doctor with gitlab? If not what options do I have?

• by ggm on 4/29/22, 9:41 PM

  Maybe catenation of an fString and a string should yield an fString by type promotion? String is morally "any" so it feels to me like a contextual narrowing of type.

• by baisq on 4/29/22, 9:49 PM

  The comments on this submission are overwhelmingly negative. Why are the comments on PVS-Studio submissions, on the other hand, generally positive?

• by safwan on 4/29/22, 10:52 PM

  f-string does not work with GNU/gettext! It is also a common mistake that people make!