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

PyInstaller Support for SunPy #5224

Merged
merged 28 commits into from
Jun 24, 2021
Merged
Show file tree
Hide file tree
Changes from 16 commits
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
b9228cf
PyInstaller Support for SunPy
jeffreypaul15 Apr 18, 2021
c3ebb25
codestyle changes
jeffreypaul15 Apr 18, 2021
17004e1
changelog added
jeffreypaul15 Apr 18, 2021
1f4d945
changed a few comments
jeffreypaul15 Apr 18, 2021
fbb7a63
Added new test hook and absolute imports
jeffreypaul15 Apr 26, 2021
bf4da06
removed license, asd skip if windows and reworked test script to run …
jeffreypaul15 Apr 26, 2021
9924d42
remvoved astroquery data files
jeffreypaul15 Apr 26, 2021
6497950
Merge branch 'master' into sunpy-5207
jeffreypaul15 Apr 26, 2021
56d76e8
codestyle
jeffreypaul15 Apr 26, 2021
b7e2022
Merge branch 'sunpy-5207' of https://github.com/jeffreypaul15/sunpy i…
jeffreypaul15 Apr 26, 2021
081351d
changed required pyinstaller version
jeffreypaul15 Apr 26, 2021
9099bbd
codestyle
jeffreypaul15 Apr 26, 2021
e6d6991
add to azure pipelines
jeffreypaul15 Apr 26, 2021
5f8da30
codestyle
jeffreypaul15 Apr 26, 2021
93a1c4f
added import and changed name of test
jeffreypaul15 Apr 27, 2021
9221125
codestyle test fix
jeffreypaul15 Apr 27, 2021
555e993
re-added extras=['all'] and updated to use PyInstaller from git
jeffreypaul15 Apr 28, 2021
cfb5c9f
changed deps for pyinstaller
jeffreypaul15 Apr 28, 2021
e4716a6
Update changelog/5224.feature.rst
jeffreypaul15 Apr 28, 2021
2ac7896
Merge branch 'master' of https://github.com/jeffreypaul15/sunpy into …
jeffreypaul15 May 4, 2021
288e46c
removed import to see error
jeffreypaul15 May 4, 2021
d0085e0
redid changes
jeffreypaul15 May 7, 2021
b936a28
hidden import for skimage
jeffreypaul15 May 7, 2021
5be57b2
Merge branch 'main' of https://github.com/sunpy/sunpy into sunpy-5207
jeffreypaul15 May 8, 2021
c1a0288
removed skimage hiddenimport and installed hooks from github
jeffreypaul15 May 8, 2021
8b92179
modified hooks for sunpy and separated test imports
jeffreypaul15 May 8, 2021
eadc990
modified hooks for sunpy and separated test imports
jeffreypaul15 May 8, 2021
21580e0
rebased
jeffreypaul15 Jun 20, 2021
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
5 changes: 5 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -232,6 +232,11 @@ docs/guide/data_types/figure.png
*.orig
.tmp

# PyInstaller Files
.pyinstaller/run_sunpy_tests
.pyinstaller/sunpy_tests
.pyinstaller/run_sunpy_tests.spec

### Vagrant: https://www.toptal.com/developers/gitignore/api/vagrant
# General
.vagrant/
Expand Down
14 changes: 14 additions & 0 deletions .pyinstaller/hooks/hook-sunpy.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
from PyInstaller.utils.hooks import collect_data_files, collect_entry_point, collect_submodules, copy_metadata

datas, hiddenimports = collect_entry_point("pytest11")

datas += collect_data_files("sunpy")
datas += collect_data_files("dask")
datas += collect_data_files("drms")
datas += copy_metadata("sunpy")

hiddenimports += collect_submodules('sunpy')
hiddenimports += collect_submodules('numpy.distutils')
hiddenimports += collect_submodules('distutils')
hiddenimports += ['skimage.filters.rank.core_cy_3d']
Copy link
Member

Choose a reason for hiding this comment

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

Say what now? Why is this here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I encountered these missing imports when running the SunPy's tests for PyInstaller, adding them fixed it.

Copy link
Member

Choose a reason for hiding this comment

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

I am curious as to how we got here in the import stack, I didn't realise we had even an optional transitive dependency on skimage.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

from skimage.feature import match_template

Which leads to the __init__ of filters being executed from skimage
https://github.com/scikit-image/scikit-image/blob/178e05f17855859d6661ebb6e04d8ddcb50c1cc5/skimage/filters./rank/generic_cy.pyx#L11

Because of the relative import, falls under hidden import category.
This isn't our issue as of now and a PR has just been merged over at pyinstaller/pyinstaller-hooks-contrib#107
PyInstaller combines these hooks on a monthly basis so I'm not sure how long it'd take for that to be sorted.

I've left it here for the time being, I suppose this is essential and I don't think we need it just for the tests.

hiddenimports += ['sunpy.data.data_manager.tests.mocks']
81 changes: 81 additions & 0 deletions .pyinstaller/run_sunpy_tests.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,81 @@
import os
import sys
import shutil

import pytest

# Skipping these tests that take check the name of the current module (which ends up starting with
# sunpy_tests rather than sunpy).
# asdf path issue with PyInstaller as well.
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this an asdf issue or a sunpy issue?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

An issue specifically with asdf and PyInstaller, works fine when used without.
The other tests take the wrong directory into consideration.

Copy link
Member

Choose a reason for hiding this comment

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

Is this just the tests, or will asdf fail to find the schemas and break when running user code?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Just these tests from what I've noticed. I haven't checked it thoroughly though, is there any way we can?


if getattr(sys, 'frozen', False):
# Running in a bundle
bundle_dir = sys._MEIPASS

SKIP_TESTS = [
'test_saveframe',
'test_saveframe_arr',
'test_genericmap_basic',
'test_genericmap_mask',
'test_attr_metamagic',
'test_main_nonexisting_module',
'test_main_stdlib_module',
'test_origin',
'test_find_dependencies',
'test_missing_dependencies_by_extra',
'test_hgc_100',
'test_missing_dependencies_by_extra',
'test_basic',
'test_data_manager',
'test_file_tampered',
'test_download_cache',
'test_skip_all',
'test_override_file',
'test_same_file_id_different_module']
Comment on lines +16 to +34
Copy link
Contributor

Choose a reason for hiding this comment

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

Can any of these be removed now?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

From the script, these tests either use current directory's file name which is 'sunpy_tests' instead of 'sunpy' which resutls in an assertion error.
Or this is an issue with asdf, the asdf pathing seems to work fine when run without PyInstaller


sys.exit(pytest.main(['sunpy_tests',
'-k ' + ' and '.join('not ' + test for test in SKIP_TESTS)]))
else:
ROOT = os.path.join(os.path.dirname(__file__), '../')

for root, dirnames, files in os.walk(os.path.join(ROOT, 'sunpy')):
for dirname in dirnames:
final_dir = os.path.relpath(
os.path.join(
root.replace(
'sunpy',
'sunpy_tests'),
dirname),
ROOT)
# We only copy over 'tests' directories, but not sunpy/tests (only
# sunpy/tests/tests) since that is not just a directory with tests.
if dirname == 'tests' and not root.endswith('sunpy'):
shutil.copytree(os.path.join(root, dirname), final_dir, dirs_exist_ok=True)
else:
# Create empty __init__.py files so that 'sunpy_tests' still
# behaves like a single package, otherwise pytest gets confused
# by the different conftest.py files.
init_filename = os.path.join(final_dir, '__init__.py')
if not os.path.exists(os.path.join(final_dir, '__init__.py')):
os.makedirs(final_dir, exist_ok=True)
with open(os.path.join(final_dir, '__init__.py'), 'w') as f:
f.write("#")
# Copy over all conftest.py files
for file in files:
if file == 'conftest.py':
final_file = os.path.relpath(
os.path.join(
root.replace(
'sunpy',
'sunpy_tests'),
file),
ROOT)
shutil.copy2(os.path.join(root, file), final_file)

# Add the top-level __init__.py file
with open(os.path.join('sunpy_tests', '__init__.py'), 'w') as f:
f.write("#")

# Copy the top-level conftest.py
shutil.copy2(os.path.join(ROOT, 'sunpy', 'conftest.py'),
os.path.join('sunpy_tests', 'conftest.py'))
2 changes: 2 additions & 0 deletions azure-pipelines.yml
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,8 @@ stages:
- linux: py38-conda
libraries: {}

- linux: pyinstaller


- ${{ if or(eq(variables['Build.Reason'], 'Schedule'), eq(variables['Build.Reason'], 'Manual')) }}:
- stage: CronTests
Expand Down
1 change: 1 addition & 0 deletions changelog/5224.feature.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Added hook file and tests for using PyInstaller with SunPy.
jeffreypaul15 marked this conversation as resolved.
Show resolved Hide resolved
9 changes: 7 additions & 2 deletions sunpy/coordinates/tests/test_frameattributes.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,14 @@
from astropy.time import Time

from sunpy.coordinates import frames, get_earth
from sunpy.coordinates.frameattributes import ObserverCoordinateAttribute, TimeFrameAttributeSunPy
from sunpy.coordinates.frames import (
HeliocentricInertial,
HeliographicCarrington,
HeliographicStonyhurst,
Helioprojective,
)
from sunpy.time import parse_time
from ..frameattributes import ObserverCoordinateAttribute, TimeFrameAttributeSunPy
from ..frames import HeliocentricInertial, HeliographicCarrington, HeliographicStonyhurst, Helioprojective


@pytest.fixture
Expand Down
9 changes: 7 additions & 2 deletions sunpy/coordinates/tests/test_frames.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,10 +13,15 @@
)
from astropy.tests.helper import assert_quantity_allclose

from sunpy import sun
from sunpy.coordinates.frames import (
Heliocentric,
HeliographicCarrington,
HeliographicStonyhurst,
Helioprojective,
)
from sunpy.time import parse_time
from sunpy.util.exceptions import SunpyUserWarning
from ... import sun
from ..frames import Heliocentric, HeliographicCarrington, HeliographicStonyhurst, Helioprojective
nabobalis marked this conversation as resolved.
Show resolved Hide resolved

RSUN_METERS = sun.constants.get('radius').si.to(u.m)
DSUN_METERS = sun.constants.get('mean distance').si.to(u.m)
Expand Down
6 changes: 5 additions & 1 deletion sunpy/coordinates/tests/test_wcs_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,12 @@
HeliographicStonyhurst,
Helioprojective,
)
from sunpy.coordinates.wcs_utils import (
_set_wcs_aux_obs_coord,
solar_frame_to_wcs_mapping,
solar_wcs_frame_mapping,
)
from sunpy.util import SunpyUserWarning
from ..wcs_utils import _set_wcs_aux_obs_coord, solar_frame_to_wcs_mapping, solar_wcs_frame_mapping


@pytest.mark.parametrize('ctype, frame', [[['HPLN', 'HPLT'], Helioprojective],
Expand Down
2 changes: 1 addition & 1 deletion sunpy/data/data_manager/tests/test_manager.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ def test_basic(storage, downloader, data_function):
assert Path(storage._store[0]['file_path']).name == ('sunpy.test_file')


def test_cache(manager, storage, downloader, data_function):
def test_download_cache(manager, storage, downloader, data_function):
"""
Test calling function multiple times does not redownload.
"""
Expand Down
2 changes: 1 addition & 1 deletion sunpy/data/tests/test_sample.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import pytest

from .._sample import _download_sample_data, _retry_sample_data
from sunpy.data._sample import _download_sample_data, _retry_sample_data


@pytest.mark.remote_data
Expand Down
14 changes: 1 addition & 13 deletions sunpy/io/special/asdf/tags/tests/test_coordinate_frames.py
Original file line number Diff line number Diff line change
@@ -1,17 +1,15 @@
import os
import platform
from distutils.version import LooseVersion

import numpy as np
import pytest

import asdf
import astropy.units as u
from astropy.coordinates import CartesianRepresentation

import sunpy.coordinates.frames as frames
from sunpy.tests.helpers import asdf_entry_points

asdf = pytest.importorskip('asdf', '2.0.2')
from asdf.tests.helpers import assert_roundtrip_tree # NOQA isort:skip

sunpy_frames = list(map(lambda name: getattr(frames, name), frames.__all__))
Expand Down Expand Up @@ -61,22 +59,12 @@ def test_hgc_100():
assert hgc.observer.object_name == 'earth'


# Skip these two tests on windows due to a weird interaction with atomicfile
# and tmpdir
skip_windows_asdf = pytest.mark.skipif(
(LooseVersion(asdf.__version__) < LooseVersion("2.3.1")
and platform.system() == 'Windows'),
reason="See https://github.com/spacetelescope/asdf/pull/632")


@skip_windows_asdf
@asdf_entry_points
def test_saveframe(coordframe_scalar, tmpdir):
tree = {'frame': coordframe_scalar}
assert_roundtrip_tree(tree, tmpdir)


@skip_windows_asdf
@asdf_entry_points
def test_saveframe_arr(coordframe_array, tmpdir):
tree = {'frame': coordframe_array}
Expand Down
13 changes: 0 additions & 13 deletions sunpy/io/special/asdf/tags/tests/test_genericmap.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,11 @@
isort:skip_file.
"""
# flake8: noqa: E402
import platform
from distutils.version import LooseVersion

import numpy as np
import pytest

import astropy.units as u
asdf = pytest.importorskip('asdf', '2.0')
from asdf.tests.helpers import assert_roundtrip_tree

import sunpy.map
Expand All @@ -24,15 +21,6 @@ def aia171_test_map():
return sunpy.map.Map(aia_path)


# Skip these two tests on windows due to a weird interaction with atomicfile
# and tmpdir
skip_windows_asdf = pytest.mark.skipif(
(LooseVersion(asdf.__version__) < LooseVersion("2.3.1")
and platform.system() == 'Windows'),
reason="See https://github.com/spacetelescope/asdf/pull/632")


@skip_windows_asdf
@asdf_entry_points
def test_genericmap_basic(aia171_test_map, tmpdir):

Expand All @@ -41,7 +29,6 @@ def test_genericmap_basic(aia171_test_map, tmpdir):
assert_roundtrip_tree(tree, tmpdir, extensions=SunpyExtension())


@skip_windows_asdf
@asdf_entry_points
def test_genericmap_mask(aia171_test_map, tmpdir):

Expand Down
4 changes: 2 additions & 2 deletions sunpy/util/sysinfo.py
Original file line number Diff line number Diff line change
Expand Up @@ -59,10 +59,10 @@ def system_info():
"""
base_reqs = get_distribution("sunpy").requires()
base_reqs = {base_req.name.lower() for base_req in base_reqs}
extra_reqs = get_distribution("sunpy").requires(extras=["all"])
Cadair marked this conversation as resolved.
Show resolved Hide resolved
extra_reqs = get_distribution("sunpy").requires()
extra_reqs = sorted({extra_req.name.lower() for extra_req in extra_reqs}.difference(base_reqs))

missing_packages, installed_packages = find_dependencies(package="sunpy", extras=["all"])
missing_packages, installed_packages = find_dependencies(package="sunpy")
extra_prop = {"System": platform.system(),
"Arch": f"{platform.architecture()[0]}, ({platform.processor()})",
"Python": platform.python_version(),
Expand Down
21 changes: 21 additions & 0 deletions tox.ini
Original file line number Diff line number Diff line change
Expand Up @@ -179,3 +179,24 @@ install_command = pip install --no-deps {opts} {packages}
commands =
conda list
{env:PYTEST_COMMAND} {posargs}

[testenv:pyinstaller]
nabobalis marked this conversation as resolved.
Show resolved Hide resolved
description = check that sunpy can be included in a pyinstaller bundle
changedir = .pyinstaller
deps =
pyinstaller
extras =
tests
net
all
Copy link
Member

Choose a reason for hiding this comment

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

We don't need these here do we? All we are running from tox is pyinstaller? or does pyinstaller copy from your current env?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Tox needs these to ensure that the tests run fine by itself, PyInstaller then copies the current environment (tox) and creates the executable based on the environment it was running on.

Copy link
Contributor

@nabobalis nabobalis Apr 28, 2021

Choose a reason for hiding this comment

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

Do we still need all of these now?

all, encompasess net. can we get away with just all and test?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

All doesn't seem to load Beautiful Soup which is part of net, this gave me an error locally I might need to check what else is required from tests and add them individually

Copy link
Contributor

Choose a reason for hiding this comment

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

It should, I wonder what the cause is.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

My apologies, must've been some local issue, we don't need net

commands =
./python3 run_sunpy_tests.py
pyinstaller --onefile run_sunpy_tests.py \
--distpath . \
Cadair marked this conversation as resolved.
Show resolved Hide resolved
--additional-hooks-dir hooks \
--exclude-module tkinter \
--hidden-import pytest \
--hidden-import sunpy \
--hidden-import pytest_mpl.plugin

./run_sunpy_tests
Cadair marked this conversation as resolved.
Show resolved Hide resolved