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

Fix stginga to handle changes to the Ginga help system in release v5.0 #234

Merged
merged 5 commits into from
Feb 15, 2024
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 5 additions & 5 deletions .github/workflows/ci_workflows.yml
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ jobs:
with:
fetch-depth: 0
- name: Set up Python
uses: actions/setup-python@v4
uses: actions/setup-python@v5
with:
python-version: '3.x'
- name: Lint with flake8
Expand All @@ -70,9 +70,9 @@ jobs:
with:
fetch-depth: 0
- name: Set up Python
uses: actions/setup-python@v4
uses: actions/setup-python@v5
with:
python-version: '3.8'
python-version: '3.11'
- name: Install and build
run: |
python -m pip install --upgrade pip setuptools wheel
Expand All @@ -89,7 +89,7 @@ jobs:
with:
fetch-depth: 0
- name: Set up Python
uses: actions/setup-python@v4
uses: actions/setup-python@v5
with:
python-version: '3.x'
- name: Install and build
Expand All @@ -109,7 +109,7 @@ jobs:
- name: Checkout code
uses: actions/checkout@v4
- name: Set up Python
uses: actions/setup-python@v4
uses: actions/setup-python@v5
with:
python-version: '3.x'
- name: Install and build
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/predeps_workflow.yml
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ jobs:
with:
fetch-depth: 0
- name: Set up Python
uses: actions/setup-python@v4
uses: actions/setup-python@v5
with:
python-version: '3.11'
- name: Install and build
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/publish-to-pypi.yml
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ jobs:
with:
fetch-depth: 0

- uses: actions/setup-python@v4
- uses: actions/setup-python@v5
with:
python-version: '3.x'

Expand Down
9 changes: 9 additions & 0 deletions CHANGES.rst
Original file line number Diff line number Diff line change
@@ -1,6 +1,15 @@
1.5 (unreleased)
----------------

Other Changes and Additions
^^^^^^^^^^^^^^^^^^^^^^^^^^^

- Compatibility with Ginga 5. [#234]

- This version requires Python 3.9 or later.
Also bumped minimum versions of other dependencies to
``astropy>=5``, ``ginga>=4.1``, and ``scipy>=1``. [#234]

1.4 (2023-11-28)
----------------

Expand Down
2 changes: 1 addition & 1 deletion docs/stginga/install.rst
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ Installation

``stginga`` requires:

* Python 3.8 or later
* Python 3.9 or later
* Astropy
* SciPy
* Ginga, see `Ginga's documentation <https://ginga.readthedocs.io/>`_
Expand Down
8 changes: 4 additions & 4 deletions setup.cfg
Original file line number Diff line number Diff line change
Expand Up @@ -35,10 +35,10 @@ packages = find:
zip_safe = False
install_requires =
numpy
astropy>=3
ginga>=2.7
scipy>=0.18
python_requires = >=3.8
astropy>=5
ginga>=4.1
scipy>=1
python_requires = >=3.9

[options.extras_require]
test =
Expand Down
27 changes: 17 additions & 10 deletions stginga/plugins/local_plugin_mixin.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,20 +26,27 @@
class HelpMixin(object):
def help(self):
"""Display online help for the plugin."""
if not self.fv.gpmon.has_plugin('WBrowser'):
pllim marked this conversation as resolved.
Show resolved Hide resolved
self._help_docstring()
return
if self.fv.gpmon.has_plugin('WBrowser') and Widgets.has_webkit:
Copy link
Contributor

Choose a reason for hiding this comment

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

My clean installation didn't pick up webkit. I cannot find any python-webkit on PyPI. How do I install it?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

In conda it can be installed by pyqtwebengine. It looks like PyPI has PyQtWebEngine, but I would install it via the same package manager as you installed pyqt otherwise you are likely to run into problems. This is one reason I want to get rid of it--it doesn't seem to be installed by default when one installs pyqt.

# ginga < v5.x
self.fv.start_global_plugin('WBrowser')

# need to let GUI finish processing, it seems
self.fv.update_pending()

self.fv.start_global_plugin('WBrowser')
obj = self.fv.gpmon.get_plugin('WBrowser')

# need to let GUI finish processing, it seems
self.fv.update_pending()
# Unlike Ginga, we do not attempt to download offline doc
# but just point to online doc directly.
obj.browse(self.help_url)
return

obj = self.fv.gpmon.get_plugin('WBrowser')
if not hasattr(self.fv, 'help_plugin'):
Copy link
Contributor

Choose a reason for hiding this comment

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

With Ginga 4, because I don't have python-webkit, I hit this line but only if I do not have docutils installed. I noticed it because the terminal complains about 'publish_string' is not defined.

However, when I install docutils, I do not hit this, instead Ginga now tries to fire up WBrowser and complains that I do not have webkit. 🤯

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Right, the web browser widget does not seem to be installed by default when you install qt, and unfortunately, in Ginga 4 this does not disable WBrowser. Instead, when you start WBrowser, it tells you to install webkit to get things working. (Qt has changed the name of their version of webkit and in Qt 5 it's name is qtwebengine AFAIK).

Without a working WBrowser, in Ginga 4 it will attempt to fall back to _help_docstring() which in Ginga 4 will attempt to use docutils (another thing I am getting rid of). I've updated this PR so that it will use an stginga version of _help_docstring, which will punt directly to showing the RST text. Give it a try and LMK how it goes.

# ginga < v5.x but somehow user doesn't have WBrowser plugin
self._help_docstring() # works same as in v4.x
return

# Unlike Ginga, we do not attempt to download offline doc
# but just point to online doc directly.
obj.browse(self.help_url)
# ginga v5.x
self.fv.help_plugin(self, url=self.help_url)


class MEFMixin(object):
Expand Down
Loading