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 issue #98 #99

Merged
merged 1 commit into from Jun 12, 2023
Merged

Fix issue #98 #99

merged 1 commit into from Jun 12, 2023

Conversation

lohedges
Copy link
Contributor

@lohedges lohedges commented Jun 8, 2023

This PR closes #98 by updating the expected box angles used in a test to those that are now generated by the triclinic lattice reduction following this fix. While I could generate the values automatically using BioSimSpace.Box, I'd rather have them written in so that it's easier for a user to debug. (Also, these box parameters weren't generated using functionality from that sub-package.)

  • 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 temporarily deployed to biosimspace-build June 8, 2023 14:03 — with GitHub Actions Inactive
@lohedges lohedges requested a review from chryswoods June 8, 2023 14:03
@lohedges lohedges temporarily deployed to biosimspace-build June 8, 2023 14:03 — with GitHub Actions Inactive
@lohedges lohedges temporarily deployed to biosimspace-build June 8, 2023 14:03 — with GitHub Actions Inactive
@lohedges lohedges temporarily deployed to biosimspace-build June 8, 2023 14:03 — with GitHub Actions Inactive
@lohedges lohedges temporarily deployed to biosimspace-build June 8, 2023 15:47 — with GitHub Actions Inactive
@lohedges lohedges temporarily deployed to biosimspace-build June 8, 2023 15:47 — with GitHub Actions Inactive
@lohedges lohedges temporarily deployed to biosimspace-build June 8, 2023 15:47 — with GitHub Actions Inactive
@lohedges lohedges temporarily deployed to biosimspace-build June 8, 2023 15:47 — with GitHub Actions Inactive
@lohedges
Copy link
Contributor Author

lohedges commented Jun 8, 2023

Looks like there's an issue with one of our dependencies on Linux py38. Will do the usual trick of trying again in the morning before investigating further.

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.

Good idea to have them written. Also prevents a false negative if something broke in both the generator and consumer code.

@lohedges lohedges temporarily deployed to biosimspace-build June 9, 2023 14:35 — with GitHub Actions Inactive
@lohedges lohedges temporarily deployed to biosimspace-build June 9, 2023 14:35 — with GitHub Actions Inactive
@lohedges lohedges temporarily deployed to biosimspace-build June 9, 2023 14:35 — with GitHub Actions Inactive
@lohedges lohedges temporarily deployed to biosimspace-build June 9, 2023 14:35 — with GitHub Actions Inactive
@lohedges lohedges temporarily deployed to biosimspace-build June 9, 2023 14:35 — with GitHub Actions Inactive
@lohedges
Copy link
Contributor Author

lohedges commented Jun 9, 2023

Yes, worked fine today 🤷‍♂️

@lohedges lohedges merged commit 3f1b8c1 into devel Jun 12, 2023
5 checks passed
@lohedges lohedges deleted the fix_98 branch June 12, 2023 11:07
lohedges added a commit that referenced this pull request Jun 12, 2023
lohedges added a commit that referenced this pull request Jun 12, 2023
Backport fixes from PR #95 and #99 into main
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] Angles will need to be updated in tests/_SireWrappers/test_system.py::test_set_box
2 participants