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 issues #215 and #216 #217

Merged
merged 2 commits into from
Dec 8, 2023
Merged

Fix issues #215 and #216 #217

merged 2 commits into from
Dec 8, 2023

Conversation

lohedges
Copy link
Contributor

@lohedges lohedges commented Dec 6, 2023

This PR closes #215 and closes #216, fixing issues that were picked up when running the funnel metadynamics tutorial here. The fixes have been tested by running the same tutorial.

  • I confirm that I have merged the latest version of devel into this branch before issuing this pull request (e.g. by running git pull origin devel): [y]
  • I confirm that I have permission to release this code under the GPL3 license: [y]

Suggested reviewers:

@chryswoods

@lohedges lohedges added the bug Something isn't working label Dec 6, 2023
@lohedges lohedges requested a review from chryswoods December 6, 2023 15:56
@lohedges lohedges temporarily deployed to biosimspace-build December 6, 2023 15:56 — with GitHub Actions Inactive
@lohedges lohedges temporarily deployed to biosimspace-build December 6, 2023 15:56 — with GitHub Actions Inactive
@lohedges lohedges temporarily deployed to biosimspace-build December 6, 2023 15:56 — with GitHub Actions Inactive
@lohedges lohedges temporarily deployed to biosimspace-build December 6, 2023 15:56 — with GitHub Actions Inactive
@lohedges lohedges temporarily deployed to biosimspace-build December 6, 2023 15:56 — with GitHub Actions Inactive
Copy link
Contributor

@chryswoods chryswoods left a comment

Choose a reason for hiding this comment

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

Looks good. I agree that try_import shouldn't be used there. We will eventually need to go through and work out when we do want to use try_import and when we don't. This would be part of a larger bit of work through to make dependencies more modular (or create a "minimal" install). It is a complex problem to tackle though, especially as dependencies of dependencies (especially ML) are getting bigger and ever more interconnected.

@lohedges lohedges merged commit d9569df into devel Dec 8, 2023
5 checks passed
@lohedges lohedges deleted the fix_215_216 branch December 8, 2023 20:59
lohedges added a commit that referenced this pull request Dec 11, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
2 participants