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.

Sign up for GitHub

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

Jump to bottom

Clean up some low-hanging warnings in test suite #310

Merged
merged 12 commits into from
Jul 24, 2024
Merged

Clean up some low-hanging warnings in test suite #310

merged 12 commits into from
Jul 24, 2024

Conversation

ajjackson
Copy link
Collaborator

@ajjackson ajjackson commented Jul 24, 2024 •
edited
Loading

  • Spglib 2.5 deprecation warnings; when caused by Euphonic, we can solve the compatibility issue by converting to a dict. When caused by a dependency, suppress the warning.
  • Filter out expected warnings from certain tests in a more systematic way
  • Avoid a Matplotlib warning when calling tight_layout() on a plot with a colourbar... by not doing that.
  • Replace noisy implicit filling of masked values with NaN with explicit filling

There is a divide-by-zero warning which I think has its roots in nasty pint/spectroscopy units hackery. That is not just an artifact and should be dealt with in a separate PR.

@ajjackson
Spglib 2.5 compatibility in brille.py
dedc337 
Spglib 2.5 introduces dataclasses for many of its data objects, and
raises a DeprecationWarning if you try to use it as a dictionary.

That's very nice, but we'd like to support a range of versions,
so for now we convert the new classes to a dict. We can migrate when
these versions of Spglib are more established.
@ajjackson
Suppress Spglib 2.5 deprecation warning raised by seekpath
e8a0e7f 
Maybe when seekpath fixes this we can bump both version requirements?
@ajjackson
Catch warning during intensity-map broadening test
a1de124
@ajjackson
Integrate warning check into main test for optimise_dipole_parameter
5edbbc6 
The warning was still raised in the main test. In order to suppress
this without suppressing unexpected warnings, we may as well check it
raised correctly. The other test is then fully redundant.
@ajjackson
Call to tight_layout() raises warning but seems to be unnecessary
a605cb2
@ajjackson
Explicitly fill masked array values with NaN
8bcd317 
Otherwise a warning is raised.
@ajjackson
Account for warning raised in TestGetQptLabels
c9140bc
@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Jul 24, 2024 •
edited
Loading

Test Results

    22 files  ± 0      22 suites  ±0   29m 53s ⏱️ -29s
 1 048 tests  -  2   1 042 ✅  -  2   6 💤 ±0  0 ❌ ±0 
10 420 runs   - 20  10 357 ✅  - 20  63 💤 ±0  0 ❌ ±0 

Results for commit bbf31ca. ± Comparison against base commit 8c87ed6.

This pull request removes 37 and adds 35 tests. Note that renamed tests count towards both. 
euphonic_test.test_qpoint_frequencies.TestQpointFrequenciesCalculateDosMap ‑ test_calculate_dos_map[NaCl-NaCl_band_yaml_from_phonopy_qpoint_frequencies.json-ebins2-NaCl_band_yaml_dos_map.json]
euphonic_test.test_qpoint_frequencies.TestQpointFrequenciesCalculateDosMap ‑ test_calculate_dos_map[quartz-quartz_bandstructure_cv_only_qpoint_frequencies.json-ebins0-quartz_bandstructure_dos_map.json]
euphonic_test.test_qpoint_frequencies.TestQpointFrequenciesCalculateDosMap ‑ test_calculate_dos_map[quartz-quartz_bandstructure_cv_only_qpoint_frequencies.json-ebins1-quartz_bandstructure_dos_map_uneven_bins.json]
euphonic_test.test_qpoint_frequencies.TestQpointFrequenciesGetDispersion ‑ test_get_dispersion[NaCl-NaCl_band_yaml_from_phonopy_qpoint_frequencies.json-NaCl_band_yaml_dispersion.json]
euphonic_test.test_qpoint_frequencies.TestQpointFrequenciesGetDispersion ‑ test_get_dispersion[quartz-quartz_bandstructure_cv_only_qpoint_frequencies.json-quartz_bandstructure_cv_only_dispersion.json]
euphonic_test.test_qpoint_frequencies.TestQpointFrequenciesGetDispersion ‑ test_get_dispersion[quartz-quartz_bandstructure_qpoint_frequencies.json-quartz_bandstructure_dispersion.json]
euphonic_test.test_spectrum1d.TestSpectrum1DMethods ‑ test_broaden[args0-methane_pdos_index_1.json-methane_pdos_index_1_1meV_gauss_broaden.json]
euphonic_test.test_spectrum1d.TestSpectrum1DMethods ‑ test_broaden[args1-quartz_666_dos.json-quartz_666_1meV_gauss_broaden_dos.json]
euphonic_test.test_spectrum1d.TestSpectrum1DMethods ‑ test_broaden[args2-quartz_666_dos.json-quartz_666_1meV_gauss_broaden_dos.json]
euphonic_test.test_spectrum1d.TestSpectrum1DMethods ‑ test_broaden[args3-quartz_666_dos.json-quartz_666_1meV_lorentz_broaden_dos.json]
…

euphonic_test.test_qpoint_frequencies.TestQpointFrequenciesCalculateDosMap ‑ test_calculate_dos_map[NaCl-NaCl_band_yaml_from_phonopy_qpoint_frequencies.json-ebins2-NaCl_band_yaml_dos_map.json-context2]
euphonic_test.test_qpoint_frequencies.TestQpointFrequenciesCalculateDosMap ‑ test_calculate_dos_map[quartz-quartz_bandstructure_cv_only_qpoint_frequencies.json-ebins0-quartz_bandstructure_dos_map.json-context0]
euphonic_test.test_qpoint_frequencies.TestQpointFrequenciesCalculateDosMap ‑ test_calculate_dos_map[quartz-quartz_bandstructure_cv_only_qpoint_frequencies.json-ebins1-quartz_bandstructure_dos_map_uneven_bins.json-context1]
euphonic_test.test_qpoint_frequencies.TestQpointFrequenciesGetDispersion ‑ test_get_dispersion[NaCl-NaCl_band_yaml_from_phonopy_qpoint_frequencies.json-NaCl_band_yaml_dispersion.json-context2]
euphonic_test.test_qpoint_frequencies.TestQpointFrequenciesGetDispersion ‑ test_get_dispersion[quartz-quartz_bandstructure_cv_only_qpoint_frequencies.json-quartz_bandstructure_cv_only_dispersion.json-context1]
euphonic_test.test_qpoint_frequencies.TestQpointFrequenciesGetDispersion ‑ test_get_dispersion[quartz-quartz_bandstructure_qpoint_frequencies.json-quartz_bandstructure_dispersion.json-context0]
euphonic_test.test_spectrum1d.TestSpectrum1DMethods ‑ test_broaden[args0-methane_pdos_index_1.json-methane_pdos_index_1_1meV_gauss_broaden.json-context0]
euphonic_test.test_spectrum1d.TestSpectrum1DMethods ‑ test_broaden[args1-quartz_666_dos.json-quartz_666_1meV_gauss_broaden_dos.json-context1]
euphonic_test.test_spectrum1d.TestSpectrum1DMethods ‑ test_broaden[args2-quartz_666_dos.json-quartz_666_1meV_gauss_broaden_dos.json-context2]
euphonic_test.test_spectrum1d.TestSpectrum1DMethods ‑ test_broaden[args3-quartz_666_dos.json-quartz_666_1meV_lorentz_broaden_dos.json-context3]
…

♻️ This comment has been updated with latest results.

@ajjackson
Clean up warnings from Spectrum2D
8cdacf2 
To deal with broadening bin warnings, this uses an elegant approach to
error/warning "expectations" suggested by the pytest docs.

We find that different warning messages are raised by ostensibly the
same problem, depending on the code path. That's not ideal, but can
stay for now.

A unit-strip warning in test_mul is also fixed.
@ajjackson
Add warning checks to test_qpoint_frequencies
dfe4e1f
@ajjackson
Integrate warning check to spectrum1d broadening test
3863e1f 
Testing separately results in an un-caught warning. May as well check
in one place.
@ajjackson
Make warning checks in test suite more consistent
7cc01e8 
- use "context" as name; the word "expected" appears enough already

- use dummy context manager does_not_raise to simplify implementation
  in test_util
@ajjackson ajjackson marked this pull request as ready for review July 24, 2024 15:02
@ajjackson ajjackson added the nice to have Not urgently required, but would be an improvement label Jul 24, 2024
@ajjackson ajjackson added this to the v1.4.0 milestone Jul 24, 2024
@ajjackson ajjackson merged commit 75fe42e into master Jul 24, 2024
15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
nice to have Not urgently required, but would be an improvement
Projects
None yet
Milestone
v1.4.0
Development

Successfully merging this pull request may close these issues.
1 participant
@ajjackson