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

Convert project to a python package (library) #6

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

gsalvatella
Copy link
Contributor

The current project is meant to be used as a python library by external applications (i.e. importing its modules). However, it is not correctly implemented in the form of a package and subpackages. This creates several problems when trying to import it from the outside.

For example, sys.path must be used to tell python where the modules are located, and the import paths used in the project itself may conflict when sourced from outside the top directory. Other paths such as the location of resource files is also broken when trying to import the project from the outside (see customer case 00304836). In fact, the project itself must make use of sys.path for the exact same reasons, which is far from ideal.

The solution is to convert the project to a package that external applications can import directly. Ideally, the package should be available in the public PyPI repository for automatic installation as a dependency. This PR starts the process by adding the corresponding __init__.py files and restructuring the import paths.

Add `__init__.py` files to the top directory and subpackage folders to
convert the project into a packageable python library. Modify the import
paths accordingly. Since python package names should not contain dashes,
rename the existing folders with underscores.

NOTE: ideally, the whole project should be packaged and uploaded to the
PyPi repository for external projects to import it as a dependency. This
would require the addition of a `pyproject.toml` and a pipeline that
builds and publishes the package accordingly.
The PDS definition file used as a template for configuring the WFX chip
is currently sourced from a hardcoded path. This path is not valid when
the library is imported from an outside project and has to be manually
renamed. This leads to several compatibility problems and is considered
bad practice. Instead, source the file through the `importlib.resources`
module provided by the standard library. This ensures that the file will
be transparently sourced regardless of the library installation path.
@MarcDorval
Copy link
Collaborator

MarcDorval commented Jul 27, 2023

Thanks for this work.
Building Python packages is not something we're familiar with, so we definitely can use a little help making the project easier to import. We'll need some time to test it, and will merge if it works fine.

@gsalvatella
Copy link
Contributor Author

Sure no problem. I could assist you on this. We're using the project extensively for our product certification. If you decide to go serious and fully convert this to a public package, the next step would be to add a pyproject.toml and a pipeline that automatically builds and publishes the package on e.g. each release. An advantage of this approach is that the package automatically pulls all dependencies (such as paramiko, pyserial, etc), so you don't need to manually document their installation process in the READMEs.

@gsalvatella
Copy link
Contributor Author

@MarcDorval what is the status of this, do you see a chance to merge it?

@gsalvatella
Copy link
Contributor Author

gsalvatella commented Nov 21, 2024

@MarcDorval our team is starting a new project to create a GUI that makes use of this library. It would be very beneficial to push this forward. Consider that having this project as a library would make things as simple as pip install wfx-common-tools, and in any python file:

import wfx-common-tools

What do you think? If you set green light I can quickly rebase this PR.

@MarcDorval
Copy link
Collaborator

I need to get my ideas clear on this, given that GitHub mentions conflicts in pds_compress.py and wfx_test_target.py.
If you can work out some changes on your side to avoid conflicts it would probably be faster (I haven't been using this in months).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants