-
Notifications
You must be signed in to change notification settings - Fork 227
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
Add template files to package #347
Conversation
Specify all HTML files under templates folder to be included as package data files.
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 |
@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? |
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 wrote:
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 |
I was affected by #345. I executed:
And it fixed the error on my designs (I didn't test examples).
|
@formatc1702 - maybe you can do a quick test similar to #347 (comment) at your macOS platform to verify that this fix is sufficient to install the templates also at your platform? |
I have had a look at the files used to publish v0.4 to PyPI and can confirm that they are missing the I can also confirm that adding this fix resolves the issue. My (simpler) test method:
Indeed, adding and removing the line in this proposed fix adds/removes the |
Specify all HTML files under templates folder to be included as package data files.
I still have this problem after installing wireviz today with macos. The template directory doesn't exist. Any suggestions? |
@aricbeaver wrote:
This PR is merged into the |
Specify all HTML files under templates folder to be included as package data files.
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 others could confirm at different platforms and versions.Please, anyone, test this branch at the platform(s) you have access to, and report back here the output from all commands. See the simple installation command in #347 (comment) below.
Alternatively, a longer and more complicated test procedure is suggested a bit earlier below, and if you don't have
git
installed, you can replace thegit clone
command with unpacking all files from this ZIP-file into a new subfolder you callWireViz
using your favorite unzip tool. The subfolder is calledWireViz-fix345
inside the ZIP-file, but you can rename it after unpacking.