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

Fix/ include dependencies in packaging - issue #169 #170

Draft
wants to merge 66 commits into
base: master
Choose a base branch
from

Conversation

AngRodrigues
Copy link
Member

Description

The license, readme and dependencies.txt file should be included in conda packaging. These files are not currently being packaged with conda.
This prevents the DependencyChecker class to work properly, because it can't find the dependencies.txt

Proposed solution in this PR:

  • ensure that the files are packaged using the build.sh and bld.bat strategy from here
  • also update the dependencies in conda recipe to the same as in dependencies.txt

Fixes #169

Type of change

  • Bug fix (non-breaking change which fixes an issue)

How Has This Been Tested?

  • Confirmed that the artifacts here have the 3 files packaged in each OS.
  • Installed from tar.bz2 in windows and confirmed that dependencies.txt is available from site_packages
  • DependencyChecker works as expected (after adjusting the path to the right location after install).

@AngRodrigues AngRodrigues marked this pull request as draft January 8, 2025 00:06
@AngRodrigues
Copy link
Member Author

While testing the builds, noticed that there is an issue with the conda install geopandas for python=3.8.
Solution was to adapt the install step in the map2loop actions with the suggested geopandas install here.

all builds seem to be passing now, including for python=3.8, so this PR is ready for review again

@AngRodrigues AngRodrigues marked this pull request as ready for review January 8, 2025 00:31
.github/workflows/conda.yml Show resolved Hide resolved
setup.py Outdated Show resolved Hide resolved
conda/build.sh Outdated Show resolved Hide resolved
@AngRodrigues AngRodrigues marked this pull request as draft January 9, 2025 00:44
@AngRodrigues
Copy link
Member Author

After digging a bit, I realized that installation with pip install . and pip install <wheel> will use different builds (and hence we need the dependencies and the data specified both in the setup.py and in the pyproject.toml; the manifest.in is also required for pip install .
There is also a difference between files available in the package source files, and the files that are available via site-packages after installation. Because dependencies.txt are in the root, they are not available in the site-packages after installation. So I added custom builds both in setup.py and conda to copy the license, readme and dependencies.txt to the root/map2loop folder so they are available through site-packages after install.
Perhaps an easier solution would've been to change the location of dependencies to root/map2loop, but this would possibly involve changing the ci, docs builds, etc, so I figured it would be best to just keep the custom builds, as they seem to be working.

I tested both the py wheel and the conda install and the DependencyChecker works as expected.
Will merge this tomorrow.

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.

[Bug] - License, readme and dependencies not packaged
2 participants