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

Conda migration #559

Closed
6 tasks done
gcroci2 opened this issue Jan 19, 2024 · 8 comments · Fixed by #565, #611 or #615
Closed
6 tasks done

Conda migration #559

gcroci2 opened this issue Jan 19, 2024 · 8 comments · Fixed by #565, #611 or #615
Assignees
Labels
CI continuous integration disc Discussion needed, to better define the task/s

Comments

@gcroci2
Copy link
Collaborator

gcroci2 commented Jan 19, 2024

After PR #549, DSSP installation is done via conda. Now the goal is to further clean the installation of deeprank2 and deploy it on conda.

@gcroci2 gcroci2 added the priority Solve this first label Jan 19, 2024
@gcroci2 gcroci2 self-assigned this Jan 19, 2024
@gcroci2 gcroci2 added CI continuous integration disc Discussion needed, to better define the task/s and removed priority Solve this first labels Jan 19, 2024
@gcroci2 gcroci2 linked a pull request Feb 6, 2024 that will close this issue
@gcroci2
Copy link
Collaborator Author

gcroci2 commented Mar 1, 2024

I used grayskull to generate the meta.yml file, pointing to the latest PyPI installation of the package.

  • Reasons for not using the PyPI release but directly the Github repo (the latter is also possible with grayskull, see here:
    • It seems more convenient to use PyPI (e.g., repositories are usually larger than tarballs, draining shared CI time and bandwidth, repositories are not checksummed and thus using a tarball has a stronger guarantee that the download that is obtained to build from is in fact the intended package). This of course mean that we can't stop releasing there.
  • We are now investigating how to include deps that are on conda, but not on conda-forge. Relevant links:
  • Running the tests locally gives an error. Since we have deps not on conda-forge is expected, but the error seems related to something else. See here
  • Not related to the recipe, but more (I think) to the PyPI release, all the tests/ folders are included in the PyPI release

Copy link

This issue is stale because it has been open for 30 days with no activity.

@github-actions github-actions bot added the stale issue not touched from too much time label Apr 12, 2024
@gcroci2 gcroci2 added the blocking This issue blocks the others label Jun 6, 2024
@gcroci2
Copy link
Collaborator Author

gcroci2 commented Jun 17, 2024

Since the issue discussed in conda/conda-build#532 remains unresolved and there has been no activity on it for a year, we have decided to revert to using pip for installation. We will continue to use conda only for packages that specifically require it.

@gcroci2 gcroci2 linked a pull request Jun 17, 2024 that will close this issue
@DaniBodor
Copy link
Collaborator

Since the issue discussed in conda/conda-build#532 remains unresolved and there has been no activity on it for a year, we have decided to revert to using pip for installation. We will continue to use conda only for packages that specifically require it.

Does this mean this issue can be closed? Or is there still some action needed from this issue?

Before closing, maybe it would make sense to also explain in this issue why poetry wouldn't work either. Just in case in the future someone wants to reconsider moving to either conda or poetry, then the information of why it doesn't presently work is all consolidated in one place.

@github-actions github-actions bot removed the stale issue not touched from too much time label Jun 19, 2024
@gcroci2
Copy link
Collaborator Author

gcroci2 commented Jun 20, 2024

Since the issue discussed in conda/conda-build#532 remains unresolved and there has been no activity on it for a year, we have decided to revert to using pip for installation. We will continue to use conda only for packages that specifically require it.

Does this mean this issue can be closed? Or is there still some action needed from this issue?

I am reverting things in #611

Before closing, maybe it would make sense to also explain in this issue why poetry wouldn't work either. Just in case in the future someone wants to reconsider moving to either conda or poetry, then the information of why it doesn't presently work is all consolidated in one place.

Good point, I'll add it here

@gcroci2
Copy link
Collaborator Author

gcroci2 commented Jun 20, 2024

A note about why we didn't used poetry.

Poetry is designed to work with pip dependencies, not conda packages. This is a significant limitation for this project since it relies on several conda packages from specific channels.
Our current setup (a yml file for conda dependencies and pip for the rest) is straightforward and easy to understand. Users familiar with conda and pip will find this approach intuitive.

@gcroci2 gcroci2 removed the blocking This issue blocks the others label Jun 20, 2024
@gcroci2 gcroci2 closed this as completed Jul 1, 2024
@gcroci2 gcroci2 moved this to Done in Development Jul 12, 2024
@DaniBodor
Copy link
Collaborator

@gcroci2 , had you looked at uv at all as an alternative to the conda migration? I just came across it (same developers as ruff) and thought that that might be a nice solution. I'm just not sure whether it will ultimately have the same issues as poetry does (I tried scanning through the issues, but couldn't see anything about it).

@gcroci2
Copy link
Collaborator Author

gcroci2 commented Nov 20, 2024

@gcroci2 , had you looked at uv at all as an alternative to the conda migration? I just came across it (same developers as ruff) and thought that that might be a nice solution. I'm just not sure whether it will ultimately have the same issues as poetry does (I tried scanning through the issues, but couldn't see anything about it).

Seems very interesting! We should definitely keep it in mind. But for this package, I don't think it solves the issue about the many conda deps we have (from what I've read it doesn't support conda installations) @DaniBodor

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CI continuous integration disc Discussion needed, to better define the task/s
Projects
Status: Done
2 participants