search

waleedka / hiddenlayer

Neural network graphs and training metrics for PyTorch, Tensorflow, and Keras.
MIT License
1.8k stars 267 forks source link

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

Closed philferriere closed 6 years ago

philferriere commented 6 years ago

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

waleedka commented 6 years ago

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 commented 6 years ago

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.