WireViz icon indicating copy to clipboard operation
WireViz copied to clipboard

Add template files to package

Open kvid opened this issue 1 year ago • 1 comments

Specify all HTML files under templates folder to be included as package data files.

Is this sufficient to fix #345? Some sources claim that include_package_data=True also is needed, but preliminary testing at my Windows setup seems to prove otherwise. I would prefer that some ohers could confirm at different platforms and versions.

kvid avatar May 16 '24 14:05 kvid

Suggested test procedure:

cd {some empty folder}
python3 -m venv .venv
source .venv/bin/activate
git clone https://github.com/wireviz/WireViz --branch=fix345
cd WireViz
pip install .
cp examples/demo0?.yml ..
cd ..
rm -rf WireViz
wireviz demo0?.yml
which wireviz
pip -V
python -V
uname -a

or at Windows:

cd {some empty folder}
python3 -m venv .venv
call .venv/Scripts/activate.bat
git clone https://github.com/wireviz/WireViz --branch=fix345
cd WireViz
pip install .
copy examples/demo0?.yml ..
cd ..
rmdir /s /q WireViz
wireviz demo0?.yml
where wireviz
pip -V
python -V
ver

kvid avatar May 19 '24 01:05 kvid

@amotl - Thank's for a super quick approval. At which platform(s) did you test the installation? Or did you just review the changed line and approved it based on earlier experience that this should work without actually testing it?

kvid avatar Jun 04 '24 17:06 kvid

Yeah, I did not explicitly test it. But in general, it looks good to me, and I assume you've tested it on your workstation?

amotl avatar Jun 04 '24 17:06 amotl

@amotl wrote:

Yeah, I did not explicitly test it. But in general, it looks good to me, and I assume you've tested it on your workstation?

As I wrote in the description at the top, I've tested it at my Windows-setup. However, I've not much experience in testing installation scripts, and I've asked for help to test at different platforms, so I would be glad if you can test at your platform(s) independently and verify that my change is sufficient. Please also simplify or otherwise improve my suggested test procedure.

About the added line in setup.py, do you know any better documentation of setup keyword arguments than https://docs.python.org/3.8/distutils/setupscript.html ? Is there a recommended or established usual order of the arguments?

kvid avatar Jun 04 '24 18:06 kvid

I was affected by #345. I executed:

pip uninstall wireviz
pip3 install git+https://github.com/wireviz/WireViz@fix345

And it fixed the error on my designs (I didn't test examples).

~/.local/bin/wireviz
pip 23.3.2 from /usr/lib/python3.12/site-packages/pip (python 3.12)
Python 3.12.3
Linux prc1 6.8.11-300.fc40.x86_64 #1 SMP PREEMPT_DYNAMIC Mon May 27 14:53:33 UTC 2024 x86_64 GNU/Linux

KarolNi avatar Jun 06 '24 12:06 KarolNi

@formatc1702 - maybe you can do a quick test similar to https://github.com/wireviz/WireViz/pull/347#issuecomment-2152259121 at your macOS platform to verify that this fix is sufficient to install the templates also at your platform?

kvid avatar Jun 07 '24 15:06 kvid

I have had a look at the files used to publish v0.4 to PyPI and can confirm that they are missing the templates/ directory.

I can also confirm that adding this fix resolves the issue.

My (simpler) test method:

  • run python -m build in the project root directory
  • check the contents of the .whl (actually a zip file) and .tar.gz files generated inside the dist/ directory. This is what I use to publish to PyPI.

Indeed, adding and removing the line in this proposed fix adds/removes the templates/ folder within the build files. I did not have to use include_package_data=True.

17o2 avatar Jun 07 '24 16:06 17o2

I still have this problem after installing wireviz today with macos. The template directory doesn't exist. Any suggestions?

aricbeaver avatar Jul 04 '24 22:07 aricbeaver

@aricbeaver wrote:

I still have this problem after installing wireviz today with macos. The template directory doesn't exist. Any suggestions?

This PR is merged into the release/v0.4.1-rc branch of PR #365 that is not yet released, but describes how you can install the fixes before the release.

kvid avatar Jul 05 '24 10:07 kvid