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

tests failing with jdaviz main #92

Closed
Tracked by #68
kecnry opened this issue Feb 23, 2024 · 4 comments
Closed
Tracked by #68

tests failing with jdaviz main #92

kecnry opened this issue Feb 23, 2024 · 4 comments

Comments

@kecnry
Copy link
Member

kecnry commented Feb 23, 2024

devdeps tests are recently failing in PRs with jdaviz main for viewers and markers. It's unclear whether this change is expected and needs to be fixed here or is a sign of a bug in jdaviz that has been introduced but not caught by upstream test coverage.

@pllim
Copy link
Contributor

pllim commented Feb 26, 2024

lcviz/tests/test_plugin_markers.py::test_plugin_markers -
AssertionError: assert ('Cursor 0.00... 1.00497e+00') == ('Cursor 0.00... 9.67587e-01')
  
  At index 1 diff: 'Time 0.00000e+00 d' != 'Time 5.45833e+00 d'
  
  Full diff:
    (
        'Cursor 0.00000e+00, 0.00000e+00',
  -     'Time 5.45833e+00 d',
  ?           ^ ^^^^^
  +     'Time 0.00000e+00 d',
  ?           ^ ^^^^^
  -     'Flux 9.67587e-01',
  +     'Flux 1.00497e+00',
    )
FAILED lcviz/tests/test_viewers.py::test_reset_limits - assert 0.9675873265993092 == 0.0

Why does this look familiar. Didn't you change how the viewer limits work recently, Kyle?

If you cannot remember a bisect of Jdaviz main would be your best bet.

@pllim
Copy link
Contributor

pllim commented Feb 26, 2024

Maybe this is what I am thinking about. Does this PR affect you?

@kecnry
Copy link
Member Author

kecnry commented Feb 26, 2024

It should not, this test is for a scatter (light curve) viewer.

EDIT: turns out it was to blame anyways.

@kecnry
Copy link
Member Author

kecnry commented Mar 28, 2024

Fixed by kecnry#8 for jdaviz 3.9 and #102 for lcviz main (to avoid testing with jdaviz 3.9 in devdeps)

@kecnry kecnry closed this as completed Mar 28, 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

No branches or pull requests

2 participants