You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
At the moment, pynsee is written with one file per class or function, with the files having the same name.
This seem to lead to name shadowing upon import (cf. #173) [1] and makes imports extremely cumbersome.
In my opinion, there is no good reason to have one file per function (especially not with the same name) as any good IDE will automatically locate definitions (even GitHub's web UI does this).
A simple way to do this with minimal changes would be to remove duplicate functions (no need to have get_geodata calling _get_geodata). So we can just keep _get_geodata.py which would contain get_geodata removing both one file, one function call, and the weird name shadowing issue.
For classes, we should follow the python convention and keep class files lowercase.
[1]: though this is not supposed to happen (declared variables are supposed to have precedence over modules) so I'm not really sure why this led to bugs in the CI
The text was updated successfully, but these errors were encountered:
I agree that it is not really 'beautiful', the main reason of this split is to avoid circular dependencies. If you manage to put the functions next to another without any circular dependency then go for it!
Roger that!
I'm not really worried about the beauty so much as about this very puzzling import issue so if we can simplify import and make sure that we don't run into this anymore, it would be a plus.
I'll have a look at it whenever I can.
In the meantime, I think we can at least move class files to lowercase, that won't hurt.
At the moment,
pynsee
is written with one file per class or function, with the files having the same name.This seem to lead to name shadowing upon import (cf. #173) [1] and makes imports extremely cumbersome.
In my opinion, there is no good reason to have one file per function (especially not with the same name) as any good IDE will automatically locate definitions (even GitHub's web UI does this).
A simple way to do this with minimal changes would be to remove duplicate functions (no need to have
get_geodata
calling_get_geodata
). So we can just keep_get_geodata.py
which would containget_geodata
removing both one file, one function call, and the weird name shadowing issue.For classes, we should follow the python convention and keep class files lowercase.
[1]: though this is not supposed to happen (declared variables are supposed to have precedence over modules) so I'm not really sure why this led to bugs in the CI
The text was updated successfully, but these errors were encountered: