glue-viz / glue-wwt

WorldWideTelescope viewer in glue
BSD 3-Clause "New" or "Revised" License
2 stars 6 forks source link

Remove `asynchronous=False` from runJavaScript calls #84

Open dhomeier opened 2 years ago

dhomeier commented 2 years ago

Description

Currently causing failures

>       self.viewer._wwt.widget.page.runJavaScript("tourxml = '';", asynchronous=False)
E       TypeError: runJavaScript() got an unexpected keyword argument 'asynchronous'

on all Python 3.7+ tests although all the Qt* modules seem to be on the same versions as in the 3.6 envs. Search of the QtWebEngine docs did not bring up anything about a deprecation or potential replacement, rather appears it is now running asynchronously by default, so just giving this a try.

codecov[bot] commented 2 years ago

Codecov Report

Merging #84 (3298584) into main (608d879) will increase coverage by 1.27%. The diff coverage is 100.00%.

:exclamation: Current head 3298584 differs from pull request most recent head 5774409. Consider uploading reports for the commit 5774409 to get more accurate results

@@            Coverage Diff             @@
##             main      #84      +/-   ##
==========================================
+ Coverage   71.97%   73.25%   +1.27%     
==========================================
  Files          18       18              
  Lines         860      860              
==========================================
+ Hits          619      630      +11     
+ Misses        241      230      -11     
Impacted Files Coverage Δ
glue_wwt/viewer/tools.py 71.69% <100.00%> (+20.75%) :arrow_up:
glue_wwt/viewer/tests/test_wwt_widget.py 97.47% <0.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 608d879...5774409. Read the comment docs.

astrofrog commented 2 years ago

@dhomeier is this still needed? can you rebase?

dhomeier commented 2 years ago

@dhomeier is this still needed? can you rebase?

Attempting to run without asynchronous=False is obviously not a solution, but restoring the kwarg and the test test_save_tour now fails on all CI platforms. So this particular PR is probably of no further use; could just leave #85 open. But I think that means SaveTourTool now has to be considered nonfunctional with any more recent JS installations.

dhomeier commented 2 years ago

There may be some useful hints in https://stackoverflow.com/a/60831141 for implementing an alternative.