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 PyOxidizer #1923

Closed
wants to merge 9 commits into from

Conversation

duytnguyendtn
Copy link
Collaborator

Description

This PR encompasses the learning lessons I've made to package jdaviz in pyoxidizer. Requires #1890

Some background:
PyOxidizer's approach is much more understandable to me. The end product is similar to PyInstaller in that it is intended to produce a portable executable that can be carried around. PyOxidizer encapsulates a python shell, along with all the dependencies you designate, into a portable package, and you can specify different modules, commands, and instructions to execute in the python shell when you "run" the executable.

So if you just package an "empty" executable, you'll get a portable python instance you can carry around with you! Then if you install different packages, you effectively have a portable virtual environment.

The main blocker I've run into is indygreg/PyOxidizer#237. jsonschema which is used by asdf a direct dependency of jdaviz invokes __package__ to determine the location of the schema folder in the jsonschema installation in lib/site-packages. The folder exists and is installed by PyOxidizer properly, but the location is different than what jsonschema is expecting. This is a known issue from PyOxidizer (as referenced above), and jsonschema is already explicitly reported as an "upstream failure" by PyOxidizer: indygreg/PyOxidizer#171

Traceback
Traceback (most recent call last):
  File "runpy", line 187, in _run_module_as_main
  File "runpy", line 146, in _get_module_details
  File "runpy", line 110, in _get_module_details
  File "jsonschema", line 23, in <module>
  File "jsonschema.validators", line 432, in <module>
  File "jsonschema._utils", line 61, in load_schema
  File "importlib.abc", line 378, in read_text
  File "importlib._adapters", line 54, in open
ValueError
error: cargo run failed

We should monitor indygreg/PyOxidizer#237 to see if this gets unblocked at some point.

One other issue I ran into: for some reason the pyzmq "subpackage" pyzmq.libs is missing from the install, despite pyzmq being installed properly. I just copied my copy from my working venv into PyOxidizer's lib folder, and it stopped complaining

Change log entry

  • Is a change log needed? If yes, is it added to CHANGES.rst? If you want to avoid merge conflicts,
    list the proposed change log here for review and add to CHANGES.rst before merge. If no, maintainer
    should add a no-changelog-entry-needed label.

Checklist for package maintainer(s)

This checklist is meant to remind the package maintainer(s) who will review this pull request of some common things to look for. This list is not exhaustive.

  • Are two approvals required? Branch protection rule does not check for the second approval. If a second approval is not necessary, please apply the trivial label.
  • Do the proposed changes actually accomplish desired goals? Also manually run the affected example notebooks, if necessary.
  • Do the proposed changes follow the STScI Style Guides?
  • Are tests added/updated as required? If so, do they follow the STScI Style Guides?
  • Are docs added/updated as required? If so, do they follow the STScI Style Guides?
  • Did the CI pass? If not, are the failures related?
  • Is a milestone set? Set this to bugfix milestone if this is a bug fix and needs to be released ASAP; otherwise, set this to the next major release milestone.
  • After merge, any internal documentations need updating (e.g., JIRA, Innerspace)?

@pllim
Copy link
Contributor

pllim commented Feb 10, 2023

I think we agreed that this isn't the installer we were looking for.

@pllim pllim closed this Feb 10, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants