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 dependency resolution issues for non-locked installations of CytoTable in tests #217

Merged
merged 3 commits into from
Jul 12, 2024

Conversation

d33bs
Copy link
Member

@d33bs d33bs commented Jul 10, 2024

Description

This PR seeks to resolve a Poetry installation issue with CytoTable where dependency resolutions cause GitHub Actions jobs to hang during the poetry install step. I've noticed that this occurs due to botocore versioning and resolution challenges and that this can be avoided by specifying a botocore version within the pyproject.toml. We saw this occur with an un-locked installation of CytoTable here. I tested a fix for this on a forked branch with an un-locked install here. Cloudpathlib utilizes botocore to assist with S3-based compatibility.

I'm unsure if this should be persisted to non-development installations of CytoTable. As it stands, this will help pass tests here but it's possible others could run into this as well. The challenge with specifying a version for non-development installations could be that someone might need another version for their work which satisfies cloudpathlib's requirement but is different from what we specify here. Open to thoughts about whether this should be a requirement for installation via PyPI, etc.

Thank you for any thoughts or feedback you may have!

What is the nature of your change?

  • Bug fix (fixes an issue).
  • Enhancement (adds functionality).
  • Breaking change (fix or feature that would cause existing functionality to not work as expected).
  • This change requires a documentation update.

Checklist

Please ensure that all boxes are checked before indicating that a pull request is ready for review.

  • I have read the CONTRIBUTING.md guidelines.
  • My code follows the style guidelines of this project.
  • I have performed a self-review of my own code.
  • I have commented my code, particularly in hard-to-understand areas.
  • I have made corresponding changes to the documentation.
  • My changes generate no new warnings.
  • New and existing unit tests pass locally with my changes.
  • I have added tests that prove my fix is effective or that my feature works.
  • I have deleted all non-relevant text in this pull request template.

@d33bs d33bs marked this pull request as ready for review July 11, 2024 10:58
@d33bs d33bs requested review from kenibrewer and gwaybio July 11, 2024 10:59
Copy link
Member

@kenibrewer kenibrewer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems like a good approach to me. Hopefully we don't see any errors crop up on the non-development front.

@d33bs
Copy link
Member Author

d33bs commented Jul 12, 2024

Thank you @kenibrewer ! Merging this in.

@d33bs d33bs merged commit 7c191ec into cytomining:main Jul 12, 2024
12 checks passed
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