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

Bug fixes and testing #386

Merged
merged 11 commits into from
May 16, 2024
Merged

Bug fixes and testing #386

merged 11 commits into from
May 16, 2024

Conversation

jmmshn
Copy link
Collaborator

@jmmshn jmmshn commented May 16, 2024

Minor fixes

  1. Scene._repr_mimebundle_ calls self.as_dict. Since Scene is not MSONable this breaks the code.

  2. Allow display_color to be set on a subset of atoms. If the display_color is set only on a subset of atoms, pymatgen will set the rest to None, in those cases we should still use the defaults.

  3. Display MSONables. There were a couple of typo bugs in the Jupyter code for MSONables.

  • if self.get_scene -> hasattr(self, "get_scene")
  • For many objects, the as_dict results need to be jsantized before rendering.
  1. Moved app testing imports into test functions.

Local installations can run into weird issues since we are supporting so many different codes. Namely installing IFermi on Mac is sometimes problematic due to FFTW package. Moving the imports inside the test allows you to avoid these imports easily using pytest -k

  1. Moved unused tests into the root/tests directory and updated them to work with the current code.

  2. Added test for the interaction between the patched MSONable objects and Ipython

@mkhorton
Copy link
Member

Thanks for patch, I believe you still have write access so please merge when ready!

@jmmshn
Copy link
Collaborator Author

jmmshn commented May 16, 2024

Thanks, @mkhorton, will merge once I figure out the tests.
It's been a while since I looked at this code and some things have changed quite a bit.

It looks like my test failures are related to the dash_duo setup which I'm pretty unfamiliar with.
@janosh can you direct me where how to fix this?

=========================== short test summary info ============================
FAILED crystal_toolkit/apps/examples/tests/test_bandstructure.py::test_bs - AssertionError: Unexpected browser logs=None
assert None == []
FAILED crystal_toolkit/apps/examples/tests/test_basic.py::test_hello_scientist - AssertionError: Unexpected browser logs=None
assert None == []
FAILED crystal_toolkit/apps/examples/tests/test_basic.py::test_hello_structure - AssertionError: Unexpected browser logs=None
assert None == []
FAILED crystal_toolkit/apps/examples/tests/test_basic.py::test_hello_structure_interactive - AssertionError: Unexpected browser logs=None
assert None == []
FAILED crystal_toolkit/apps/examples/tests/test_diffraction.py::test_diffraction - AssertionError: Unexpected browser logs=None
assert None == []
FAILED crystal_toolkit/apps/examples/tests/test_fermi_surface.py::test_diffraction - AssertionError: Unexpected browser logs=None
assert None == []
FAILED crystal_toolkit/apps/examples/tests/test_structure.py::test_structure - AssertionError: Unexpected browser logs=None
assert None == []
============================== 7 failed in 40.18s ==============================
Error: Process completed with exit code 1.

@jmmshn
Copy link
Collaborator Author

jmmshn commented May 16, 2024

nvm it's just due to some drift in behavior of dash things I guess.
This brings up another question: do we need to lock down versions of dependencies for testing?

@jmmshn jmmshn merged commit e444bba into materialsproject:main May 16, 2024
5 checks passed
@jmmshn jmmshn mentioned this pull request May 16, 2024
@jmmshn jmmshn changed the title Patch 1 Bug fixes and testing May 16, 2024
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