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

Keep ___init__.py empty? (Codebase merge issue 1 of 3) #1

Closed
philferriere opened this issue Jun 1, 2018 · 2 comments
Closed

Keep ___init__.py empty? (Codebase merge issue 1 of 3) #1

philferriere opened this issue Jun 1, 2018 · 2 comments

Comments

@philferriere
Copy link
Collaborator

Hey @waleedka,
Please see:
https://github.com/waleedka/weightwatcher/blob/aa8f1546f27401b61296d96be8095e290318f8b3/ww/__init__.py#L2-L18

@waleedka
Copy link
Owner

waleedka commented Jun 2, 2018

We don't expect both frameworks to be installed. Should probably do conditional imports to import only what's available.

I put the import in init to simplify importing and calling the functions. The user shouldn't need to specifically import PyTorch or TF specific modules. I would keep the imports in init but update the other modules to not throw an error if a framework they don't need is not available.

@philferriere
Copy link
Collaborator Author

I'm personally a big fan of the following notation:

from ww import builder_tf

It doesn't require that you explicitly import anything in __init__.py. You can test this with the new setup.py file I've included. Just run the following command to install the package in your environment:

$ pip install .
Processing e:\repos\ww-dev
Building wheels for collected packages: weightwatcher
  Running setup.py bdist_wheel for weightwatcher ... done
  Stored in directory: C:\Users\Phil\AppData\Local\pip\Cache\wheels\17\e2\0c\1237efb07e9b9cdd2a3e1621a699435738275e3fcb15108535
Successfully built weightwatcher
distributed 1.21.8 requires msgpack, which is not installed.
Installing collected packages: weightwatcher
Successfully installed weightwatcher-0.1

It also doesn't introduce the problem of having to deal with conditional imports. It also doesn't make the code blow-up just by importing the package. Of course, no matter what, if the relevant DL framework is not installed, you will get into trouble when importing the ww TF or PyTorch builder.

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