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

POC: Attempt to package jdaviz in Pyinstaller (take 2) #1960

Merged
merged 89 commits into from
Jul 5, 2023
Merged
Show file tree
Hide file tree
Changes from 34 commits
Commits
Show all changes
89 commits
Select commit Hold shift + click to select a range
f760572
feat: pyinstaller
duytnguyendtn Dec 2, 2022
b520aef
codesign osx
maartenbreddels May 16, 2023
58730c3
debug: try without codesign
maartenbreddels May 16, 2023
47f966b
does not need arguments
maartenbreddels Jun 6, 2023
2f9041a
use hooks
maartenbreddels Jun 6, 2023
5569a4c
also download the app
maartenbreddels Jun 6, 2023
ba8ee5a
use branch of pyinstaller
maartenbreddels Jun 6, 2023
ef4b254
better hooks
maartenbreddels Jun 13, 2023
02e7025
pin to 5.11
maartenbreddels Jun 13, 2023
b0c526d
fix: maintain symlinks by zipping, upload-artifact does not support it
maartenbreddels Jun 13, 2023
afebeee
GHA logic?
maartenbreddels Jun 13, 2023
309860a
code sign on gha
maartenbreddels Jun 13, 2023
9577b2a
run notary tool on gha
maartenbreddels Jun 13, 2023
c80aad5
fix: redo codesign after modifications
maartenbreddels Jun 13, 2023
3687266
fix: reorder zipping and notary step
maartenbreddels Jun 13, 2023
6c5da95
remove invalid symlink
maartenbreddels Jun 13, 2023
a7c7e34
make sure the program executes
maartenbreddels Jun 13, 2023
76657d1
fix path of entitlements file
maartenbreddels Jun 13, 2023
8c0e2af
add comment for hint with the osx fix
maartenbreddels Jun 13, 2023
42cfcf2
gpt assisted way of running the log tool on failure
maartenbreddels Jun 13, 2023
e7cfa49
fix syntax error
maartenbreddels Jun 13, 2023
f70642e
fix path
maartenbreddels Jun 13, 2023
c083476
comments for the future
maartenbreddels Jun 13, 2023
7c42fce
fix uuid parsing and re-zip the app after notary step
maartenbreddels Jun 13, 2023
dbd8142
fix: use ditto instead of zip
maartenbreddels Jun 13, 2023
4aa88bf
comments for the future
maartenbreddels Jun 13, 2023
42d3eb1
fix uuid for notary step
maartenbreddels Jun 13, 2023
00b1886
upload different artifact for osx
maartenbreddels Jun 13, 2023
3adfbb4
cleanup spec file and hooks, no more __pycache__ files should be incl…
maartenbreddels Jun 20, 2023
18f750d
ci: do not cancel on failure
maartenbreddels Jun 20, 2023
ab8f881
fix: mistune 3.0 needs this
maartenbreddels Jun 20, 2023
fc5fb85
make dmg instead of zip for osx
maartenbreddels Jun 20, 2023
df0ace8
BUG: Fix mouseover behavior in Cubeviz
pllim Jun 20, 2023
6a79df3
Merge pull request #2258 from pllim/fix-cubeviz-spec-mouseover
pllim Jun 20, 2023
54ae4c8
Remove change log from #2258
pllim Jun 20, 2023
8350d83
Deprecate get_subsets_from_viewer
duytnguyendtn Jun 7, 2023
4872695
Missing region index
duytnguyendtn Jun 7, 2023
83ccc6c
Fix subset args
duytnguyendtn Jun 7, 2023
4c4ef73
Remove get_data_from_viewer from imviz viewer tests
duytnguyendtn Jun 8, 2023
30c5ec8
Remove get_data_from_viewer from mosviz data loading test and sub har…
duytnguyendtn Jun 8, 2023
3638a50
Mosviz test update image truth class
duytnguyendtn Jun 8, 2023
2b1eff9
Fix incorrect viewer ref
duytnguyendtn Jun 8, 2023
4d6658d
Set Mos2Dviewer data statistic to None by default
duytnguyendtn Jun 8, 2023
f43ee63
Non-existent data check
duytnguyendtn Jun 8, 2023
a402b02
Properly deprecate getters
duytnguyendtn Jun 8, 2023
e6b9eb4
Properly check for valueError on non-existent label
duytnguyendtn Jun 9, 2023
723fe1a
Update Specviz get_data_from_viewer test
duytnguyendtn Jun 9, 2023
4d6e2a4
Rename "subset_to_apply" to "spectral_subset
duytnguyendtn Jun 9, 2023
ef6a38b
Codestyle
duytnguyendtn Jun 9, 2023
0817e83
Fix docs wording
duytnguyendtn Jun 9, 2023
8672106
Catch missed code, fix bug
pllim Jun 20, 2023
1688ec1
Retain Mosviz get_data behavior
pllim Jun 20, 2023
06f2ed0
Undo bad diff
pllim Jun 21, 2023
0fdb3bd
Merge pull request #2242 from duytnguyendtn/depsubsets
pllim Jun 21, 2023
a160c5f
MNT: Add .mailmap
pllim Jun 21, 2023
d5fbd70
DOC: Add warning about surface brightness in Simple Aperture Photomet…
pllim Jun 23, 2023
40116b9
Merge pull request #2260 from pllim/add-git-mailmap
pllim Jun 23, 2023
bc6cdee
FEAT: Annulus draw tool for Imviz (#2240)
pllim Jun 29, 2023
55a19f4
MNT: Temporarily pin voila<0.5 (#2269)
pllim Jun 29, 2023
a9f13e9
TST: Ignore DeprecationWarning from asteval (#2274)
pllim Jun 29, 2023
4018883
Add doc for windows latency issue
javerbukh Jun 30, 2023
b6feaa6
Merge pull request #2278 from javerbukh/add-windows-known-bug
pllim Jun 30, 2023
2c03ef1
Deprecate load_spectrum
duytnguyendtn Jun 29, 2023
eb62365
Update tests and docs from load_spectrum
duytnguyendtn Jun 29, 2023
91d6978
Docstring suggestions
duytnguyendtn Jul 3, 2023
40fd901
Changelog
duytnguyendtn Jul 3, 2023
a569123
Changelog
duytnguyendtn Jul 3, 2023
24ff314
Button to export Cubeviz movie (#2264)
pllim Jul 3, 2023
b365d80
Merge pull request #2273 from duytnguyendtn/specload
duytnguyendtn Jul 3, 2023
d3d2513
Add functional but ugly launcher
duytnguyendtn Jun 15, 2023
cefb486
Use launcher notebook if neither config nor filepath is specified
duytnguyendtn Jun 15, 2023
e652be8
Codestyle
duytnguyendtn Jun 15, 2023
f3bb66d
Add margins on left and right to avoid cutoff in notebook
duytnguyendtn Jun 15, 2023
dff234b
Support launcher from cli
duytnguyendtn Jun 16, 2023
ceb79b0
Codestyle
duytnguyendtn Jun 16, 2023
b980750
Remove URI from path text until implemented
duytnguyendtn Jun 16, 2023
bfa8a41
Fix standalone bug
duytnguyendtn Jun 16, 2023
73c4b6f
Specify --layout= as new required cli syntax
duytnguyendtn Jun 26, 2023
0df09d9
Remove unused import
duytnguyendtn Jun 26, 2023
507a528
Update readme to show required layout flag
duytnguyendtn Jun 26, 2023
47b50a5
Changelog
duytnguyendtn Jul 3, 2023
0cd7a80
Changelog
duytnguyendtn Jul 3, 2023
294c8d6
Changelog
duytnguyendtn Jul 3, 2023
4c859b7
Merge pull request #2257 from duytnguyendtn/launcher
duytnguyendtn Jul 3, 2023
42075d8
Update .github/workflows/standalone.yml
rosteen Jul 5, 2023
d421571
Update .github/workflows/standalone.yml
rosteen Jul 5, 2023
9e2c857
Update to use config launcher
rosteen Jul 5, 2023
c28cb2e
Merge branch 'pyinstaller_v2' of https://github.com/maartenbreddels/j…
rosteen Jul 5, 2023
946780b
Moved changelog entry to new section
rosteen Jul 5, 2023
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
141 changes: 141 additions & 0 deletions .github/workflows/standalone.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,141 @@
name: Build standalone

on:
push:
branches:
- main
- 'v*'
tags:
- 'v*'

defaults:
run:
shell: bash {0}


jobs:
build_binary:
runs-on: ${{ matrix.os }}-latest
strategy:
matrix:
os: [ubuntu, windows, macos]
steps:
# osx signing based on https://melatonin.dev/blog/how-to-code-sign-and-notarize-macos-audio-plugins-in-ci/
- name: Import Certificates (macOS)
uses: apple-actions/import-codesign-certs@v1
if: ${{ matrix.os == 'macos' }}
with:
p12-file-base64: ${{ secrets.DEV_ID_APP_CERT }}
p12-password: ${{ secrets.DEV_ID_APP_PASSWORD }}
- uses: actions/checkout@v3
with:
fetch-depth: 0

- uses: actions/setup-python@v4
with:
python-version: "3.10"

- name: Install jdaviz
run: pip install .

- name: Install pyinstaller
run: pip install pyinstaller==5.11

- name: Create standalone binary
env:
DEVELOPER_ID_APPLICATION: ${{ secrets.DEVELOPER_ID_APPLICATION }}
run: (cd standalone; pyinstaller ./jdaviz.spec)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
run: (cd standalone; pyinstaller ./jdaviz.spec)
run: (cd standalone; pyinstaller -m ./jdaviz.spec)

Cami found the executable on macos was not flagged as an executable, requiring a chmod +x. This article suggests the manifest flag should fix this: https://stackoverflow.com/questions/9969464/why-does-my-pyinstaller-created-executable-require-admin-privileges

Copy link
Collaborator

Choose a reason for hiding this comment

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

@duytnguyendtn it looks like you linked the wrong thing here, your comment points to a Jira ticket rather than an article. Any chance you have the original link?

Copy link
Collaborator

Choose a reason for hiding this comment

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

FYI this does not work as suggested, the -m argument requires a file as input.


- name: Remove invalid files for OSX
# hopefully we can improve this in the future
# by using good hooks
# i think the issue is that we have a . in the name, there are many
# google hits on pyqt having the same issue
# and we might be able to remove it after https://github.com/pyinstaller/pyinstaller/pull/7619
# is released (pyinstaller 5.13 probably)
if: ${{ matrix.os == 'macos' }}
run: |
rm -rf standalone/dist/jdaviz.app/Contents/MacOS/skimage/.dylibs
rm -rf standalone/dist/jdaviz.app/Contents/Resources/skimage/.dylibs

- name: Codesign (OSX)
if: ${{ matrix.os == 'macos' }}
run: |
cd standalone/dist
codesign --deep --force --options=runtime --entitlements ../entitlements.plist --sign ${{ secrets.DEVELOPER_ID_APPLICATION }} --timestamp jdaviz.app

- name: Create dmg (OSX)
# if we do not call always() GHA will && with success()
if: ${{ always() && (matrix.os == 'macos') }}
# it seems ditto (not zip) should be used in combination with notarization
# see https://developer.apple.com/forums/thread/116831
# but dmg also works
# see https://github.com/glue-viz/glue-standalone-apps/blob/main/.github/workflows/build_stable.yml
run: |
rm -rf standalone/dist/jdaviz
hdiutil create -volname "Jdaviz" -srcfolder standalone/dist -ov -format UDZO standalone/dist/jdaviz.dmg

- name: Notary step + stapling (OSX)
if: ${{ matrix.os == 'macos' }}
run: |
output=$(xcrun notarytool submit standalone/dist/jdaviz.dmg --apple-id ${{ secrets.NOTARIZATION_USERNAME }} --team-id ${{ secrets.TEAM_ID }} --wait --password ${{ secrets.NOTARIZATION_PASSWORD }}) || true
echo "$output"
uuid=$(echo "$output" | awk -F '[ :]+' '/id:/ {print $3; exit}')
echo "UUID: $uuid"
if [[ $output == *"status: Accepted"* ]]; then
echo "Great, notarization succeeded, staple it!"
xcrun stapler staple standalone/dist/jdaviz.dmg
else
echo "Log output for failed notarization: $uuid"
xcrun notarytool log --apple-id ${{ secrets.NOTARIZATION_USERNAME }} --team-id ${{ secrets.TEAM_ID }} --password ${{ secrets.NOTARIZATION_PASSWORD }} $uuid || true
fi

- name: Validate app (OSX)
if: ${{ matrix.os == 'macos' }}
run: |
spctl -a -vvv -t execute standalone/dist/jdaviz.app

- name: Run jdaviz cmd in background
if: ${{ matrix.os == 'macos' }}
run: ./standalone/dist/jdaviz.app/Contents/MacOS/jdaviz-cli imviz&

- name: Run jdaviz cmd in background
if: ${{ matrix.os != 'macos' }}
run: ./standalone/dist/jdaviz/jdaviz-cli imviz&

- name: Install playwright
run: (pip install playwright; playwright install chromium)

- name: Install pytest
run: pip install pytest-playwright

- name: Wait for Voila to get online
uses: ifaxity/wait-on-action@v1
with:
resource: tcp:8866
timeout: 60000

- name: Test standalone
run: (cd standalone; touch pytest.ini; JUPYTER_PLATFORM_DIRS=1 pytest test.py --video=on)

- name: Upload Test artifacts
if: always()
uses: actions/upload-artifact@v3
with:
name: test-results-${{ matrix.os }}
path: standalone/test-results

- name: Upload jdaviz standalone (non-OSX)
if: ${{ always() && (matrix.os != 'macos') }}
uses: actions/upload-artifact@v3
with:
name: jdaviz-standlone-${{ matrix.os }}
path: |
standalone/dist/jdaviz

- name: Upload jdaviz standalone (OSX)
if: ${{ always() && (matrix.os == 'macos') }}
uses: actions/upload-artifact@v3
with:
name: jdaviz-standlone-${{ matrix.os }}
path: standalone/dist/jdaviz.dmg
4 changes: 4 additions & 0 deletions CHANGES.rst
Original file line number Diff line number Diff line change
Expand Up @@ -360,6 +360,10 @@ New Features
- Model fitting: API and UI to re-estimate model parameters based on current data/subset selection.
[#1952]

- CLI launchers no longer require data to be specified [#1960]

- Added direct launchers for each config (e.g. ``specviz``) [#1960]

Cubeviz
^^^^^^^

Expand Down
23 changes: 23 additions & 0 deletions jdaviz/_astropy_init.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
# Licensed under a 3-clause BSD style license - see LICENSE.rst
import os
import sys

if not hasattr(sys.modules["__main__"], "__file__"):
# this happens under pyinstaller when running jdaviz cli
# which triggers an error in astropy, so we set it to the
# executable path of the cli executable
sys.modules["__main__"].__file__ = sys.executable


from astropy.tests.runner import TestRunner

__all__ = ['__version__', 'test']

try:
from .version import version as __version__
except ImportError:
__version__ = ''

# Create the test function for self test
test = TestRunner.make_test_runner_in(os.path.dirname(__file__))
test = TestRunner.make_test_runner_in(os.path.dirname(__file__))
27 changes: 27 additions & 0 deletions jdaviz/cli.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
# Command-line interface for jdaviz

import inspect
import os
import pathlib
import sys
Expand All @@ -10,9 +11,11 @@

from jdaviz import __version__
from jdaviz.app import _verbosity_levels
from jdaviz import configs

__all__ = ['main']

CONFIGS_DIR = str(pathlib.Path(inspect.getfile(configs)).parent)
JDAVIZ_DIR = pathlib.Path(__file__).parent.resolve()


Expand Down Expand Up @@ -144,3 +147,27 @@
main(filepaths=args.filepaths, layout=layout, instrument=args.instrument, browser=args.browser,
theme=args.theme, verbosity=args.verbosity, history_verbosity=args.history_verbosity,
hotreload=args.hotreload)

Check warning on line 150 in jdaviz/cli.py

View check run for this annotation

Codecov / codecov/patch

jdaviz/cli.py#L150

Added line #L150 was not covered by tests

def _specviz():

Check warning on line 152 in jdaviz/cli.py

View check run for this annotation

Codecov / codecov/patch

jdaviz/cli.py#L152

Added line #L152 was not covered by tests
_main(config='specviz')

Check warning on line 154 in jdaviz/cli.py

View check run for this annotation

Codecov / codecov/patch

jdaviz/cli.py#L154

Added line #L154 was not covered by tests

def _specviz2d():
_main(config='specviz2d')


def _imviz():

Check warning on line 160 in jdaviz/cli.py

View check run for this annotation

Codecov / codecov/patch

jdaviz/cli.py#L160

Added line #L160 was not covered by tests
_main(config='imviz')


def _cubeviz():

Check warning on line 164 in jdaviz/cli.py

View check run for this annotation

Codecov / codecov/patch

jdaviz/cli.py#L164

Added line #L164 was not covered by tests
_main(config='cubeviz')


def _mosviz():

Check warning on line 168 in jdaviz/cli.py

View check run for this annotation

Codecov / codecov/patch

jdaviz/cli.py#L168

Added line #L168 was not covered by tests
_main(config='mosviz')
Comment on lines +159 to +176
Copy link
Collaborator

Choose a reason for hiding this comment

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

@duytnguyendtn do these need to be changed to use layout rather than config after your work?

And @maartenbreddels, are these necessary for the stand-alone to work? I changed the standalone entry point to call main() instead of _imviz(), but I see they are used in setup.cfg although I don't fully understand what those console_scripts entry points are doing there.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Nope! In the launcher PR, the config arg just translates it to layout if provided:

jdaviz/jdaviz/cli.py

Lines 146 to 149 in 9427518

if config is None:
layout = args.layout
else:
layout = config

Copy link
Collaborator

Choose a reason for hiding this comment

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

Excellent, got it. I'll just leave this alone then!



if __name__ == "__main__":

Check warning on line 172 in jdaviz/cli.py

View check run for this annotation

Codecov / codecov/patch

jdaviz/cli.py#L172

Added line #L172 was not covered by tests
_main()
41 changes: 41 additions & 0 deletions jdaviz/configs/cubeviz/cubeviz.ipynb
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
{
"cells": [
{
"cell_type": "code",
"execution_count": null,
"metadata": {},
"outputs": [],
"source": [
"# PREFIX\n",
"from jdaviz import Cubeviz\n",
"\n",
"cubeviz = Cubeviz(verbosity='JDAVIZ_VERBOSITY', history_verbosity='JDAVIZ_HISTORY_VERBOSITY')\n",
"data_path = 'DATA_FILENAME'\n",
"if data_path:\n",
" cubeviz.load_data('DATA_FILENAME')\n",
"cubeviz.app"
]
}
],
"metadata": {
"kernelspec": {
"display_name": "Python 3",
"language": "python",
"name": "python3"
},
"language_info": {
"codemirror_mode": {
"name": "ipython",
"version": 3
},
"file_extension": ".py",
"mimetype": "text/x-python",
"name": "python",
"nbconvert_exporter": "python",
"pygments_lexer": "ipython3",
"version": "3.7.6-final"
}
},
"nbformat": 4,
"nbformat_minor": 2
}
50 changes: 50 additions & 0 deletions jdaviz/configs/default/default.ipynb
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
{
"cells": [
{
"cell_type": "code",
"execution_count": null,
"metadata": {
"scrolled": false
},
"outputs": [],
"source": [
"from jdaviz.app import Application\n",
"app = Application(configuration='default')\n",
"app.verbosity = 'JDAVIZ_VERBOSITY'\n",
"app.history_verbosity = 'JDAVIZ_HISTORY_VERBOSITY'\n",
"data_path = 'DATA_FILENAME'\n",
"if data_path:\n",
" app.load_data('DATA_FILENAME')\n",
"app"
]
},
{
"cell_type": "code",
"execution_count": null,
"metadata": {},
"outputs": [],
"source": []
}
],
"metadata": {
"kernelspec": {
"display_name": "Python 3",
"language": "python",
"name": "python3"
},
"language_info": {
"codemirror_mode": {
"name": "ipython",
"version": 3
},
"file_extension": ".py",
"mimetype": "text/x-python",
"name": "python",
"nbconvert_exporter": "python",
"pygments_lexer": "ipython3",
"version": "3.7.6"
}
},
"nbformat": 4,
"nbformat_minor": 2
}
41 changes: 41 additions & 0 deletions jdaviz/configs/imviz/imviz.ipynb
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
{
"cells": [
{
"cell_type": "code",
"execution_count": null,
"metadata": {},
"outputs": [],
"source": [
"# PREFIX\n",
"from jdaviz import Imviz\n",
"\n",
"imviz = Imviz(verbosity='JDAVIZ_VERBOSITY', history_verbosity='JDAVIZ_HISTORY_VERBOSITY')\n",
"data_path = 'DATA_FILENAME'\n",
"if data_path:\n",
" imviz.load_data('DATA_FILENAME')\n",
"imviz.app"
]
}
],
"metadata": {
"kernelspec": {
"display_name": "Python 3",
"language": "python",
"name": "python3"
},
"language_info": {
"codemirror_mode": {
"name": "ipython",
"version": 3
},
"file_extension": ".py",
"mimetype": "text/x-python",
"name": "python",
"nbconvert_exporter": "python",
"pygments_lexer": "ipython3",
"version": "3.9.1"
}
},
"nbformat": 4,
"nbformat_minor": 2
}