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

Build recipe for the project #169

Open
7 tasks done
drprojects opened this issue Nov 8, 2024 · 8 comments
Open
7 tasks done

Build recipe for the project #169

drprojects opened this issue Nov 8, 2024 · 8 comments
Labels
enhancement New feature or request help wanted Extra attention is needed

Comments

@drprojects
Copy link
Owner

drprojects commented Nov 8, 2024

βœ… Code of conduct checklist

  • 🌱 I am using the latest version of the code.
  • πŸ“™ I thoroughly went through the README, but could not find the feature I need there.
  • πŸ“˜ I thoroughly went through the tutorial slides, notebook, and video, but could not find the feature I need there.
  • πŸ“— I thoroughly went through the documentation, but could not find the feature I need there.
  • πŸ“œ I went through the docstrings and comments in the source code parts relevant to my problem, but could not find the feature I need there.
  • πŸ‘©β€πŸ”§ I searched for similar issues, but could not find the feature I need there.
  • ⭐ Since I am showing interest in the project, I took the time to give the repo a ⭐ to show support. Please do, it means a lot to us !

πŸš€ The feature, motivation and pitch

Installing the project with a simple pip install command would be ideal. Besides, having such build recipe would allow proper automatic CI and CD, which are currently lacking. Ideally, we would have a single pyproject.toml file to cover our needs, combined with a setup.py if using setuptools.

πŸ”€ Alternatives

For now, the reliance on the install.sh script works, but is not ideal. In particular:

  • FRNN is not pip-installable and requires the execution of a shell script to compile. If this is too much of an issue, we should move away from FRNN in favor of other radius-NN tools, even if at the expense of a little bit of performance. This would however require a bit of benchmarking
  • pgeof fails to install properly in a conda environment without an explicit call to a conda-install command (see Install script makes seemingly incorrect assumptions about PGEOF's environmentΒ #102)
  • parallel_cut_pursuit and grid_graph require the execution of a python script for compilation

Any attempt to improve the project's installation would need to address these 3 points.

πŸ“š Additional context

No response

@drprojects drprojects added enhancement New feature or request help wanted Extra attention is needed labels Nov 8, 2024
@drprojects
Copy link
Owner Author

@rjanvier I saw your post sk-build-core issue tracker still did not get any reply. Have you maybe found a workaround for installing pgeof from inside a conda env ? I am starting to look into options for doing proper testing and automated building for superpoint transformer. From my current understanding, the fact that installing pgeof in an environment requires a conda-install call is preventing any pip-only build recipe for the superpoint transformer, correct ?

The second limitation I see is the fact that several dependencies currently need to be compiled with shell or python scripts. Not entirely sure how to work around this with a pyproject.toml approach.

Any suggestions would be welcome !

@gvoysey
Copy link

gvoysey commented Nov 11, 2024

I install all of superpoint transformer with pip; we don't use anaconda at all.

@drprojects
Copy link
Owner Author

@gvoysey thanks for your reply !

How do you proceed for FRNN installation ? Did you move away from this dependency ? How about the cut-pursuit and grid-graph dependencies ? Did you manage to port the script parts of Γ¬nstall.shinto asetup.py` managed by setuptools ?

@gvoysey
Copy link

gvoysey commented Nov 12, 2024

we vendored and packaged all the dependencies -- frnn, prefix-sum, grid-graph, pycut-pursuit, and pgeof are all pip installable.

Installation is managed from a pyproject.toml, per PEP 517/518. I don't see any utility in new/fancy build backends, so we're using setuptools.

@drprojects
Copy link
Owner Author

drprojects commented Nov 12, 2024

Would be quite interested in how you packaged all the dependencies these, in particular FRNN and prefix-sum (all the other dependencies are no on PyPi thanks to @rjanvier). I am fairly new to python project packaging, would you mind sharing how you configured this ?

@gvoysey
Copy link

gvoysey commented Nov 12, 2024

I'll see what i can do to get you a PR for the dependencies, as well as spt itself. it'll save my fork some churn if i do the helping, heh. πŸ˜‡

@drprojects
Copy link
Owner Author

Don't worry about making a super clean PR ! Basically if you share your recipe for compiling FRNN with setuptools, ruled from a pyproject.toml / setup.py / requirements.txt, that would be awesome. Unless essential for the packaging, I do not need to know how you reorganized src/ to be honest. Then I can capitalize on this πŸ˜‰

@gvoysey
Copy link

gvoysey commented Nov 12, 2024

It is essential for the packaging, sadly. I'm happy to do it on the grounds that it improves my ability to stay in step with your upstream remote, which is significant for our team.

i'd like to include some convenience tooling for autoformatting (your template correctly uses pre-commit but i don't think you've run it) and a stab at CI/CD with nox -- probably as two additional PRs.

happy to talk offline about a strategy for this, if you like.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

2 participants