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

Added ability to adjust bead_size #1158

Merged
merged 1 commit into from
Feb 1, 2024

Conversation

jaclark5
Copy link
Contributor

PR Summary:

Fixes #1157

Allow a user to adjust the bead size in Jupyter notebook visualization. This is especially useful for CG models where the default bead size may be too small.

PR Checklist


  • Includes appropriate unit test(s)
  • Appropriate docstring(s) are added/updated
  • Code is (approximately) PEP8 compliant
  • Issue(s) raised/addressed?

@jaclark5
Copy link
Contributor Author

How can I add unit tests for features in a Jupyter notebook?

@daico007
Copy link
Member

Hi @jaclark5, good question, and I don't think we have a good way to explicitly tests those (the visualization). Currently, the py3dmol and nglview tests basically just try to make sure that we can create the visualization object without error. So in this case, I think you can just add a simple test case (maybe a box of CG beads or something that you have ready) and make sure there's no error pops up.

    @pytest.mark.skipif(not has_py3Dmol, reason="Py3Dmol is not installed")
    def test_visualize_py3dmol(self, ethane):
        py3Dmol = import_("py3Dmol")
        vis_object = ethane._visualize_py3dmol()
        assert isinstance(vis_object, py3Dmol.view)

    @pytest.mark.skipif(not has_nglview, reason="NGLView is not installed")
    def test_visualize_nglview(self, ethane):
        nglview = import_("nglview")
        vis_object = ethane._visualize_nglview()
        assert isinstance(
            vis_object.component_0, nglview.component.ComponentViewer
        )

@daico007
Copy link
Member

And those tests are currently located in test_compound.py

Copy link

codecov bot commented Nov 28, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (c684f92) 87.18% compared to head (1dcb209) 87.18%.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #1158   +/-   ##
=======================================
  Coverage   87.18%   87.18%           
=======================================
  Files          62       62           
  Lines        6492     6492           
=======================================
  Hits         5660     5660           
  Misses        832      832           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Contributor

@chrisjonesBSU chrisjonesBSU left a comment

Choose a reason for hiding this comment

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

LGTM! Thank you @jaclark5, this is an issue I've ran into before with non-atomistic compounds.

As far as tests go, it might not be possible to test that the visualized beads are correctly changing sizes since. I think it's fine to merge without any additional tests since compound visualization isn't a critical function of mBuild.

@daico007 daico007 merged commit ea6e756 into mosdef-hub:main Feb 1, 2024
17 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.

Ability to change bead size in visualization
3 participants