Replies: 1 comment
-
TRY003 - ok, if done carefully. We do not want to introduce long living references to many objects |
Beta Was this translation helpful? Give feedback.
-
TRY003 - ok, if done carefully. We do not want to introduce long living references to many objects |
Beta Was this translation helpful? Give feedback.
-
Was looking through the Dishka's sources, I think there are some things we can improve in codestyle.
First of all, I think we should ignore less Ruff warnings. In particular:
TRY003
(Avoid specifying long messages outside the exception class)The reasoning provided in Ruff docs is good enough:
Creating different exception classes also will improve readability of the code.
TC00X
(Move library into type-checking block)As Dishka is a framework, it should do its best to be as fast as possible. Imports, after all, do cost something, so I think we should do what TC00X warnings suggest: move imports of modules that are used solely for type-checking into a
if TYPE_CHECKING
block.(however, don't really know how much of a big deal that is)
PLR0913
(Too many arguments)This check is generally okay, but there are some places that require a lot of arguments. It's better to exclude them explicitly, that to exclude the whole check.
SIM114
(Merge if branches with identical bodies)Code repetition is bad. Did not find any reasoning behind exclusion of that check. I think we should definetly bring it back. Moreover, it occurrs only 3-4 times in the whole codebase, so I think it won't be a pain to fix.
PLW2901
(Redefinition of a name in loop)Also generally a good check. As with
PLR0913
, we should not ignore the whole check, but rather some places where it's okay to reassign the variable.UP038
,SIM103
- their exclusion/inclusion does not affect the results of linter in any way. They should definetly be re-included.The second point is, we can benefit of making use of wemake-python-styleguide linter. Major issues which it reveals and we totally should address are:
Too complex lines, functions and modules.
Even alone they dramatically drop the readability and, as a consequence, the ability of new contributors to onboard easily. But here, combined with lack of thorough code documentation, they become even worse.
Code overuse.
I think we can all agree that, again, code repetition is bad. We should definetly fix that.
Some weird code (like checking types with
type(...) is ...
).There is no doubt that coding in general and creating that kind of a framework especially requires some magic and weird code in some places. But the point is: it should be documented and provided with a reasoning.
and much much more small and medium-sized issues, fixing which will improve the codebase.
The final thought is that we can add more docstrings, so the project becomes more friendly to beginning contributors.
In conclusion,
Main two goals of this proposal are:
Improve codebase so it's easier for new contributors to onboard.
Improve codebase so there is less space for bugs and (potentially) performance issues.
I find them very important, especially for an open-source project.
Hope that @Tishka17 will agree with me. Although I definetly would participate in making this proposal come true, these changes could not be done without thorough consulting and discussion with the creator and "ideologist" of the framework.
Looking forward to your comments about this.
Beta Was this translation helpful? Give feedback.
All reactions