fix setup.py so pip install does not fail #3

eschmidbauer · 2023-03-20T14:43:45Z

eschmidbauer commented

2023-03-20 14:43:45 +00:00

First-time contributor

No description provided.

eschmidbauer added 1 commit 2023-03-20 14:43:46 +00:00

fix setup.py so pip install does not fail b0b3d6c626

mrq commented

2023-03-20 15:02:41 +00:00

desu you're not supposed to install DLAS as a pip package, even if setup.py "allows" you to, there's fundamental issues (from off the top of my head a month ago):

codes being named that instead of something like dl-art-school, or whatever draconian assumptions setuptool does, makes nothing actually get copied over because setup assumes
the CBT of getting setup.py to actually copy the codes folder is a chore, as a lot of folders are missing __init_.pys
even if you get that, a lot of scripts reference things assuming that codes is the root directory, rather than referincing it as a module (like tortoise does, sanely) making it a pain to even use it as a PIP module.

If you're insistent on using DLAS outside of either mrq/ai-voice-cloning or directly invoking it, consider my disgusting hotfix. PIP-ifying DLAS isn't worth the trouble when I can just clone it as a submodule and sloppily inject that way.

You could get away with an __init__.py in the root of codes (or dlas, whatever it's renamed to) to do the sys.path injection with something like a os.path.dirname(file) or something.

desu you're not supposed to install DLAS as a pip package, even if setup.py "allows" you to, there's fundamental issues (from off the top of my head a month ago): * `codes` being named that instead of something like `dl-art-school`, or whatever draconian assumptions setuptool does, makes nothing actually get copied over because setup assumes * the CBT of getting `setup.py` to actually copy the `codes` folder is a chore, as a lot of folders are missing `__init_.py`s * even if you get that, a lot of scripts reference things assuming that `codes` *is* the root directory, rather than referincing it as a module (like tortoise does, sanely) making it a pain to even use it as a PIP module. If you're insistent on using DLAS outside of either [mrq/ai-voice-cloning](https://git.ecker.tech/mrq/ai-voice-cloning) or directly invoking it, consider my [disgusting hotfix](https://git.ecker.tech/mrq/ai-voice-cloning/src/branch/master/src/train.py#L10). PIP-ifying DLAS isn't worth the trouble when I can just clone it as a submodule and sloppily inject that way. You *could* get away with an `__init__.py` in the root of `codes` (or `dlas`, whatever it's renamed to) to do the sys.path injection with something like a os.path.dirname(__file__) or something.

eschmidbauer commented

2023-03-20 15:28:43 +00:00

First-time contributor

@mrq thanks for the feedback. will come up with other solutions.

eschmidbauer closed this pull request

2023-03-20 15:28:44 +00:00

eschmidbauer deleted branch fix-pip-install

2023-03-20 15:28:49 +00:00

mrq commented

2023-03-20 15:50:48 +00:00

Some suggestions, since I forgot past-me left some hints in previous commits:

reference my previous history with setup.py (namely, the last two).
if you're going to go the lazy but effective sys.path.append(os.path.dirname(__file__)) in ./DL-Art-School/codes/__init__.py, I did have some comments in my training.py
make sure every single folder has an __init__.py in it so setuptools registers every directory.
- or if you know of a way to, in setup.py, manually recurse the entire folder to do so without relying on it to be implicitly done, you could do that.
set the package name as "DLAS" in setup.py (to avoid whatever other way to register files for setup.py, I forget).
copy ./DL-Art-School/codes/ to ./venv/lib/python3.x/site-packages/ (or .\venv\Lib\site-packages\ on Windows) as dlas, it'll make debugging the later a lot helpful.
- this is moreso if you want the busywork of making sure a lot of the scripts reference dlas as a global package instead.

Some suggestions, since I forgot past-me left some hints in previous commits: * reference my previous history with [`setup.py`](https://git.ecker.tech/mrq/DL-Art-School/commits/branch/master/setup.py) (namely, the last two). * if you're going to go the lazy but effective `sys.path.append(os.path.dirname(__file__))` in `./DL-Art-School/codes/__init__.py`, I did have some comments in my [`training.py`](https://git.ecker.tech/mrq/ai-voice-cloning/commit/bcec64af0fb188511ddd9412519dc08cb4dd486a) * make sure every single folder has an `__init__.py` in it so setuptools registers every directory. - or if you know of a way to, in setup.py, manually recurse the entire folder to do so without relying on it to be implicitly done, you could do that. * set the package name as "DLAS" in setup.py (to avoid whatever other way to register files for setup.py, I forget). * copy `./DL-Art-School/codes/` to `./venv/lib/python3.x/site-packages/` (or `.\venv\Lib\site-packages\` on Windows) as `dlas`, it'll make debugging the later a lot helpful. - this is moreso if you want the busywork of making sure a lot of the scripts reference `dlas` as a global package instead.

eschmidbauer commented

2023-03-20 18:39:30 +00:00

First-time contributor

im thinking it makes sense to break this code out into a dlas python package https://git.ecker.tech/eschmidbauer/dlas

eschmidbauer commented

2023-03-20 18:57:46 +00:00

First-time contributor

I was able to train using the dlas python package in ai-voice-cloning with the following diff

pip install git+https://git.ecker.tech/eschmidbauer/dlas.git

...

git diff src/train.py
diff --git a/src/train.py b/src/train.py
index acd15a4..4636bca 100755
--- a/src/train.py
+++ b/src/train.py
@@ -5,10 +5,11 @@ import yaml
 import datetime

 from torch.distributed.run import main as torchrun
+import dlas.train as tr^M

 # it'd be downright sugoi if I was able to install DLAS as a pip package
-sys.path.insert(0, './modules/dlas/codes/')
-sys.path.insert(0, './modules/dlas/')
+#sys.path.insert(0, './modules/dlas/codes/')^M
+#sys.path.insert(0, './modules/dlas/')^M

 # this is effectively just copy pasted and cleaned up from the __main__ section of training.py
 def train(config_path, launcher='none'):
@@ -62,7 +63,7 @@ if __name__ == "__main__":
         pass

     import torch
-    from codes import train as tr
-    from utils import util, options as option
+    import dlas.train as tr^M
+    from dlas.utils import util, options as option^M

-    train(config_path, args.launcher)
\ No newline at end of file
+    train(config_path, args.launcher)^M

I was able to train using the `dlas` python package in `ai-voice-cloning` with the following diff `pip install git+https://git.ecker.tech/eschmidbauer/dlas.git` ... ``` git diff src/train.py diff --git a/src/train.py b/src/train.py index acd15a4..4636bca 100755 --- a/src/train.py +++ b/src/train.py @@ -5,10 +5,11 @@ import yaml import datetime from torch.distributed.run import main as torchrun +import dlas.train as tr^M # it'd be downright sugoi if I was able to install DLAS as a pip package -sys.path.insert(0, './modules/dlas/codes/') -sys.path.insert(0, './modules/dlas/') +#sys.path.insert(0, './modules/dlas/codes/')^M +#sys.path.insert(0, './modules/dlas/')^M # this is effectively just copy pasted and cleaned up from the __main__ section of training.py def train(config_path, launcher='none'): @@ -62,7 +63,7 @@ if __name__ == "__main__": pass import torch - from codes import train as tr - from utils import util, options as option + import dlas.train as tr^M + from dlas.utils import util, options as option^M - train(config_path, args.launcher) \ No newline at end of file + train(config_path, args.launcher)^M ```

mrq commented

2023-03-20 19:29:08 +00:00

That's it? Wild. I'll see about replicating it when I get a moment.

mrq commented

2023-03-21 20:28:29 +00:00

Implemented in commit a4afad8837.

DLAS is still included as a submodule in mrq/ai-voice-cloning as it's easier for it to update if I push any commits to it, rather than having it re-clone everytime per update script call.

Implemented in commit a4afad8837404b6d99c0a7da0f4337da6e34fc61. DLAS is still included as a submodule in mrq/ai-voice-cloning as it's easier for it to update if I push any commits to it, rather than having it re-clone everytime per update script call.