Skip to content

Conversation

@SimonHeybrock
Copy link
Member

@SimonHeybrock SimonHeybrock commented Sep 2, 2024

This fixes a bug in a newly introduced feature in #3501. Previously it was not possible to get into such a case because hist did not support this mode of operation.

Looking at the diff in that PR
image
it is evident that I had begun changing the code, but then keep the old way of computing drop. I wonder if there is a static analysis tool that could catch something like this (the drop defined on line 579 is unused)? Isn't it in some sense similar to an "unused argument" check?

@jl-wynen
Copy link
Member

jl-wynen commented Sep 2, 2024

I had a quick look at tools to detect unused variables.

  • vulture is a dedicated tool for this purpose. But it doesn't find this case.
  • ruff's F841 doesn't find this case either.
  • Mypy doesn't complain.
  • PyCharm marks the first drop as unused.

But ruff and vulture do find an unused variable if it's name is not reused. E.g. in

def foo(x: list[int]) -> list[int]:
    asd = [2 * i for i in x]
    drop = [1, 2, 3]
    return drop

they correctly flag asd as unused. But this check is enabled in ruff in our projects.

@SimonHeybrock SimonHeybrock merged commit 057ec4e into main Sep 2, 2024
@SimonHeybrock SimonHeybrock deleted the fix-hist-coord-preserve branch September 2, 2024 08:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

3 participants