daphne-eu / daphne

DAPHNE: An Open and Extensible System Infrastructure for Integrated Data Analysis Pipelines
Apache License 2.0
67 stars 62 forks source link

Feature/daphnelib python pkg #649

Closed m-birke closed 7 months ago

m-birke commented 10 months ago

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:

  1. I moved the python source code to a daphne subdir to actually have a python package with the name 'daphne'
  2. The imports are changed to 'from daphne.... import ...'
  3. The path to the libdaphnelib.so is acquired via a env var which specifies the dir where all the necessary .so files are located
  4. The daphnelib (cpp) gets the path to the liballkernels.so as a parameter to be independent from PWD
  5. Within the src/api/python dir Python project management files are placed
  6. I changed the tmp path to /tmp/DaphneLib because if the Python 'daphne' is installed as a package we dont know the location of the files and hence cant assume the a file related path is writable

Main issue solved in source code: removal of hardcoded paths

There are more improvements possible but for this PR it is enough

pdamme commented 10 months ago

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.

pdamme commented 7 months ago

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.

m-birke commented 7 months ago

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