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.

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

Question: import style modification #28

Open
matthewcarbone opened this issue Apr 27, 2021 · 1 comment
Open

Question: import style modification #28

matthewcarbone opened this issue Apr 27, 2021 · 1 comment

Comments

@matthewcarbone
Copy link
Contributor

I am not sure which is best practice, since the way that you're handling imports does make sense. However, I think for readability, it might make more sense to precisely specify imports.

For example, consider the __init__.py in pyroved.utils,

from .coord import (generate_grid, generate_latent_grid,
                    generate_latent_grid_traversal, transform_coordinates)
from .data import init_dataloader, init_ssvae_dataloaders
from .nn import (get_activation, get_bnorm, get_conv, get_maxpool,
                 set_deterministic_mode, to_onehot)
from .prob import get_sampler
from .viz import plot_grid_traversal, plot_img_grid, plot_spect_grid

__all__ = ['generate_grid', 'transform_coordinates', 'generate_latent_grid',
           'get_sampler', 'init_dataloader', 'init_ssvae_dataloaders',
           'get_activation', 'get_bnorm', 'get_conv', 'get_maxpool',
           'to_onehot', 'set_deterministic_mode', 'get_sampler',
           'plot_img_grid', 'plot_spect_grid', 'plot_grid_traversal',
           'generate_latent_grid_traversal']

It too me a minute to figure out where set_deterministic_mode was when reading trvae.py,

from pyroved.utils import (
    generate_grid, generate_latent_grid, get_sampler,
    plot_img_grid, plot_spect_grid, set_deterministic_mode,
    to_onehot, transform_coordinates
)

In other words, I would modify the imports here to say

from pyroved.utils.nn import set_deterministic_mode

and remove the code in the __init__.py, for example. However, I'm happy to be convinced that your current way is better. What do you think?

@ziatdinovmax
Copy link
Owner

I'm OK with this change.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants