Restore .pylintrc #483
Restore .pylintrc #483
Conversation
|
|
||
| [BASIC] | ||
| # Allow redefinition of input builtins | ||
| allowed-redefined-builtins=input |
There was a problem hiding this comment.
@ndem0 this is the restore from 0.2 back to master. Since we use a lot input as kwargs (e.g. in Condition, loss_data,..) I would keep the disable property such that pylint does not complain. What do you think?
There was a problem hiding this comment.
Ok, but it's a bit dangerous to use builtin names.
There was a problem hiding this comment.
I suggest to manually disable the warnings inside the files where these are intentionally used (e.g. InputTargetCondition). What do you think @ndem0 @dario-coscia
| max-statements=50 | ||
|
|
||
| # Minimum number of public methods for a class (see R0903). | ||
| min-public-methods=0 |
There was a problem hiding this comment.
@ndem0 This was inserted because we had a lot of Codacy issues because we have many classes in Condition that are just defined without public methods. Should we keep it like this or go back and don't care about codacy ?
There was a problem hiding this comment.
I would remove, it's typically a bad design when you have classes without public methods.
eventually also Conditions will have public methods!
There was a problem hiding this comment.
Also in this case we can apply the same approach as for input. Let me know what you think @ndem0 @dario-coscia
|
All in all, in my opinion keeping the .pylintrc as it in in master branch is the safer option. However, when we intentionally break some "codacy rules" we can disable the warning inside the file |
|
Great! It's not a huge problem to break some rules intentionally, but it's safer to keep them in order to have a quick validation that random errors are not committed. |
|
If ok for you, I merge this PR |
No description provided.