[Code health] Remove Re-definition found for builtin function

kornia / kornia

Geometric Computer Vision Library for Spatial AI

https://kornia.readthedocs.io

Apache License 2.0

9.95k stars 969 forks source link

[Code health] Remove Re-definition found for builtin function #1276

Open edgarriba opened 3 years ago

edgarriba commented 3 years ago

🚀 Feature

As reported by deepsource in here we abuse from using built-in input function in our functionality.

Motivation

We target to have a clean and healthy source code free of risk.

Pitch

Replace variable names whether it makes sense e.g. for image based functionality input -> image ; in losses input -> pred (or similar)

Consider also to contribute to Kornia universe projects :)

Tutorials: our repository containing the tutorials.

shijianjian commented 3 years ago

why not? I think it is a standard in PyTorch. Also, I do not think that we will use any input anyway in our functions. So the overriding should be okay.

edgarriba commented 3 years ago

/cc @cclauss any opinions?

cclauss commented 3 years ago

I do agree with the tool… It is not considered Pythonic to shadow a builtin. It does not matter if the current code does not use input() because a future contributor might add it and then be highly confused by the results.

sesam commented 3 years ago

@shijianjian are you working on this? What needs to be done here?

The deepsource report shows 604 occurrences -- that seem like more than easy and probably better to do in pieces to not break a lot of existing open PRs (32 of them).

Then this comment https://github.com/kornia/kornia/pull/1286#discussion_r709824227 I guess is moving the deepsource count +1?

edgarriba commented 3 years ago

@sesam I’d go module by module and possibly starting with test files