search

ufal / neuralmonkey

An open-source tool for sequence learning in NLP built on TensorFlow.
BSD 3-Clause "New" or "Revised" License
410 stars 104 forks source link

Increase pylint limits for too-many-arguments and too-many-locals #770

Open jindrahelcl opened 5 years ago

jindrahelcl commented 5 years ago

We should increase pylint limits for too-many-arguments and too-many-locals (which is influenced by the arguments which count as well towards this number).

For example, the current maximum of arguments is 8. Since the ModelPart parent class has five of them, this leaves almost no space for classes that are derived from this class.

The maximum number of arguments used in neuralmonkey is in the RNN decoder constructor (23) and there is a bunch of other functions with arg number > 20. The mean argument number (of those larger than 8) is around 12-15.

I propose to raise this number globally to e.g. 15 or 20, because now the pylint warning about too many arguments is just not useful and everyone is ignoring it and the code is full with pylint: disable lines..

jindrahelcl commented 5 years ago

Pokud by šlo ten limit nastavit jen pro konstruktory, tak bych to udělal, ale nevim, jestli to jde a jak by se to dělalo. @tomasmcz ?

tomasmcz commented 5 years ago

Nemyslím si, že Python ví, co je to konstruktor :-P Podle mne by se věci, které mají dvacet argumentů, měly refaktorovat (zkusit smysluplně rozdělit na menší části, které se nakonfigurují zvlášť a pak se spojí). Ale na druhou stranu uznávám, že v Pythonu je to asi idiomatické -- když čtu dokumentaci k matplotlibu, jsou tam všude konstruktory s desítkami arguementů.

jindrahelcl commented 5 years ago

Dvacet už je na refaktor zralý, to jo, ale osm je prostě málo skoro na všechno.