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

Add template files to package #347

Open
wants to merge 1 commit into
base: release/v0.4.1-rc
Choose a base branch
from
Open

Add template files to package #347

wants to merge 1 commit into from

Conversation

kvid
Copy link
Collaborator

@kvid kvid commented May 16, 2024

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 the git clone command with unpacking all files from this ZIP-file into a new subfolder you call WireViz using your favorite unzip tool. The subfolder is called WireViz-fix345 inside the ZIP-file, but you can rename it after unpacking.

  • Verified at Windows
  • Verified at Linux
  • Verified at macOS

Specify all HTML files under templates folder
to be included as package data files.
@kvid
Copy link
Collaborator Author

kvid commented May 19, 2024

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 kvid linked an issue May 25, 2024 that may be closed by this pull request
@kvid kvid mentioned this pull request May 25, 2024
13 tasks
@kvid kvid changed the base branch from master to release/v0.4.1-rc May 25, 2024 16:38
@kvid kvid requested a review from formatc1702 June 1, 2024 01:27
@kvid kvid added the help wanted Extra attention is needed label Jun 1, 2024
@kvid kvid requested a review from amotl June 4, 2024 17:03
@kvid
Copy link
Collaborator Author

kvid commented Jun 4, 2024

@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?

@amotl
Copy link
Member

amotl commented Jun 4, 2024

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?

@kvid
Copy link
Collaborator Author

kvid commented Jun 4, 2024

@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?

@KarolNi
Copy link

KarolNi commented Jun 6, 2024

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Extra attention is needed
Projects
None yet
3 participants