Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Organize imports #105

Closed
kiudee opened this issue Mar 23, 2020 · 2 comments · Fixed by #191 · May be fixed by #115
Closed

Organize imports #105

kiudee opened this issue Mar 23, 2020 · 2 comments · Fixed by #191 · May be fixed by #115
Labels
enhancement New feature or request Priority: Medium
Milestone
1.3

Comments

@kiudee
Copy link
Owner

kiudee commented Mar 23, 2020

Currently we import all submodules in __init__:

cs-ranking/csrank/__init__.py

Lines 3 to 9 in 55396fb

from .choicefunction import *
from .core import *
from .dataset_reader import *
from .discretechoice import *
from .objectranking import *
from .tunable import Tunable
from .tuning import ParameterOptimizer

This results in the user seeing a confusing list of submodules. We should trim that by using __all__ to only import important classes and functions.

The remaining modules are still available, but hidden by default.
@kiudee kiudee added the enhancement New feature or request label Mar 23, 2020
@kiudee kiudee added this to the 1.2 milestone Mar 23, 2020
@timokau
Copy link
Collaborator

timokau commented Apr 1, 2020

For future reference, @kiudee referenced https://pypi.org/project/zimports/ in slack.

@timokau timokau mentioned this issue Apr 1, 2020
Merged
7 tasks
timokau added a commit to timokau/cs-ranking that referenced this issue Apr 1, 2020
@timokau
Declare public interfaces explicitly
346010f 
As suggested by pep8:

https://www.python.org/dev/peps/pep-0008/#public-and-internal-interfaces

Not doing so trips up flake8's F401, since the imports are not used.
Imports are an implementation detail and should not directly be relied
upon to declare the public API.

Two exceptions to this are csrank/__init__.py and
csrank/dataset_reader/__init__.py, where the API is too big. This will
likely change in the future. See kiudee#105.
timokau added a commit to timokau/cs-ranking that referenced this issue Apr 1, 2020
@timokau
Declare public interfaces explicitly
d80d647 
As suggested by pep8:

https://www.python.org/dev/peps/pep-0008/#public-and-internal-interfaces

Not doing so trips up flake8's F401, since the imports are not used.
Imports are an implementation detail and should not directly be relied
upon to declare the public API.

Two exceptions to this are csrank/__init__.py and
csrank/dataset_reader/__init__.py, where the API is too big. This will
likely change in the future. See kiudee#105.
timokau added a commit to timokau/cs-ranking that referenced this issue Apr 1, 2020
@timokau
Declare public interfaces explicitly
a16aefc 
As suggested by pep8:

https://www.python.org/dev/peps/pep-0008/#public-and-internal-interfaces

Not doing so trips up flake8's F401, since the imports are not used.
Imports are an implementation detail and should not directly be relied
upon to declare the public API.

Two exceptions to this are csrank/__init__.py and
csrank/dataset_reader/__init__.py, where the API is too big. This will
likely change in the future. See kiudee#105.
@timokau
Copy link
Collaborator

timokau commented Apr 3, 2020

After we have tackled this, we should revisit flake8's star-import related warnings F403 and F405.

This was referenced Apr 13, 2020
Merged
Draft
@kiudee kiudee modified the milestones: 1.2, 1.3 Jun 5, 2020
@kiudee kiudee mentioned this issue Apr 13, 2021
Merged
7 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
enhancement New feature or request Priority: Medium
Projects
None yet
Milestone
1.3
2 participants
@kiudee @timokau