-
Notifications
You must be signed in to change notification settings - Fork 59
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
Feature/daphnelib python pkg #649
Conversation
Thanks for this contribution, @m-birke. Looks like this will simplify using DaphneLib a lot! I will look into it in detail later this week. |
3a50a7e
to
4297474
Compare
- Docs mention that/how DaphneLib can be used from source without installing the package. - tmpdaphne.daphne is stored in /tmp/DaphneLib, not in the pwd. - Updated the version number of the Python package to the most recent DAPHNE release version number. - Fixed some typos. - Added missing license headers.
4297474
to
cd6496a
Compare
Hi @m-birke, sorry for the delay... This contribution looks very good to me. I've just polished a few little things (see the commit I added), such as:
I think it would be great to add the installation and little test use of DaphneLib to our GitHub actions to always test if it still works as described in the docs. I would recommend a package installation from the already cloned source then, since installing from the GitHub repo seems to clone the LLVM submodule, which is quite huge. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See the above comment for my review.
Hi @pdamme thanks for the review and the additional fixes. Especially saving "tmpdaphne.daphne" to the new tmp dir as well is important takeaway for me: add github workflow for actually building and testing the package (or general python related ci/cd), i will create an issue for this KR |
Building the Python DaphneLib as a package
Now independent from CWD of python3 cmd execution when running a DLIB script
Build artifact producable which can be distributed
(potentially can be separated into different git repo now very easily)
I had to touch 90 files for this PR which is a lot, let me point reviewers here to the important changes:
Main issue solved in source code: removal of hardcoded paths
There are more improvements possible but for this PR it is enough