Parse top-level import name from wheel file by ryanking13 · Pull Request #3006 · pyodide/pyodide

ryanking13 · 2022-08-23T04:31:16Z

Description

Changes how imports key in repodata.json is calculated. This key is used in loadPackagesFromImport to get the top-level import name of a package.

Previously we were using test/imports key in meta.yaml to calculate this key, without mentioning this in docs. Therefore it was very easy to make a mistake such as #3001.

This PR changes this by calculating top-level imports dynamically from the wheel file:

1) Try to parse the top-level import name from top_level.txt file inside the wheel file. This file exists if the package is built with setuptools, but may not exist if the package is built with flit, poetry, etc.
2) Iterate the top-level directories and files inside the wheel file, and add to top-level import if there are directories containing python file or if there is a top-level python file.
3) If both of the above fails, use the package name inside meta.yaml as a top-level import name (e.g. shared libraries)

~~FYI these packages don't have top_level.txt:~~

~~- packages that use flit: pyparsing, more_itertools, tomli, tomli_w, threadpoolctl, typing_extensions~~
~~- packages that use hatchling: jsonschema, soupsieve,~~
~~- packages that use poetry: pkgconfig
~~- packages that use distutils: distlib (changed to setuptools in latest release)~~

Checklists

Add a CHANGELOG entry
Add / update tests

rth

Thanks! The nice thing is it also catches private imports (e.g. _pytest). It's an interesting approach, but note that are a number of problematic imports it finds below. Some are easy to fix, others are harder. Also this is hard to notice / review for users so I wonder whether the current system (but possibly with auto-generated list of imports by mkpkg using this code) so it could be reviewed would be more reliable.

test and tests should be explicitly excluded,

Not sure about those lib* folders, but maybe not critical,

I stopped at "l" letter in repodata.json. I imagine there are more of the same types of issues later.

pyodide-build/pyodide_build/buildall.py

ryanking13 · 2022-08-23T08:30:06Z

but note that are a number of problematic imports it finds below. Some are easy to fix, others are harder.

Right... it seems like the top_level.txt is not very reliable so probably we should iterate directories even if top_level.txt exist. Thanks for pointing that out.

so I wonder whether the current system (but possibly with auto-generated list of imports by mkpkg using this code) so it could be reviewed would be more reliable.

Yeah, that would be more flexible.

ryanking13 · 2022-08-23T08:35:59Z

The issue with auto-generating top level imports in mkpkg is that the final directory structure is not visible until we build a wheel.
Probably we can download the wheel for other platforms and use its structure.

ofek · 2022-08-23T17:49:49Z

Iterate the top-level directories and files inside the wheel file, and add to top-level import if there are directories containing __init__py or if there is a top-level python file.

Don't forget about implicit namespace packages https://packaging.python.org/en/latest/guides/packaging-namespace-packages/#native-namespace-packages

ryanking13

Updated such as:

Add a new key package/top-level in meta.yaml which is used for top-level import names.
package/top-level is auto calculated in mkpkg command. This is done by downloading a wheel file of a native platform using its directory structure. (I hope the directory structure doesn't vary between platforms...)
If a wheel file doesn't exist, it will add a placeholder so one can manually add top-level import names.

Also I stopped using top_level.txt since it often contains invalid import names as pointed out by Roman.

To make a review easier, I added comments on packages that have changes.

ryanking13 · 2022-08-24T00:11:56Z

packages/attrs/meta.yaml

  version: 21.4.0
+  top-level:
+    - attr
+    - attrs


Added attrs

ryanking13 · 2022-08-24T00:12:04Z

packages/biopython/meta.yaml

  version: "1.79"
+  top-level:
+    - Bio
+    - BioSQL


Added BioSQL.

ryanking13 · 2022-08-24T00:13:19Z

packages/matplotlib/meta.yaml

  version: 3.5.2
-
+  top-level:
+    - pylab


matplotlib has a top-level file pylab.py. Not sure about the usage of it.

ryanking13 · 2022-08-24T00:14:53Z

packages/pycryptodomex/meta.yaml

  name: pycryptodomex
  version: 3.15.0
+  top-level:
+    - Cryptodome


Intestingly, this file was named wrong (meta.yml), so this package was not built.

It seems like the build is faling, disabled for now.

ryanking13 · 2022-08-24T00:15:48Z

packages/pyrsistent/meta.yaml

  name: pyrsistent
  version: 0.18.1
+  top-level:
+    - _pyrsistent_version


It has a top-level version file...

ryanking13 · 2022-08-24T00:16:37Z

packages/ruamel.yaml/meta.yaml

@@ -1,6 +1,8 @@
 package:
-  name: ruamel
+  name: ruamel.yaml


ruamel.yaml is a correct package name.

ryanking13 · 2022-08-24T00:18:10Z

packages/scikit-image/meta.yaml

  version: 0.19.3
+  top-level:
+    - skimage
+    # - doc  # scikit-image has top level `doc` directory... but it is too common name so we don't want to use it as a hint for top-level import.


scikit-image have top-level doc directory, but the name doc is too common so I think we don't want to load scikit-image is someone do import doc.

Reported to scikit-image/scikit-image#6505

ryanking13 · 2022-08-24T00:18:45Z

packages/setuptools/meta.yaml

  name: setuptools
  version: 62.6.0
+  top-level:
+    - _distutils_hack


setuptools has _distutils_hack.

ryanking13 · 2022-08-24T00:19:16Z

packages/sympy/meta.yaml

  name: sympy
  version: 1.10.1
+  top-level:
+    - isympy


sympy has top-leve isympy.py file.

ryanking13 · 2022-08-24T00:19:32Z

packages/toolz/meta.yaml

  name: toolz
  version: 0.11.2
+  top-level:
+    - tlz


Added tlz.

hoodmane · 2022-09-02T17:13:23Z

Sorry for the delay on review for this @ryanking13. I'll try to look at it when I get a chance. Seems like it's in good shape.

rth

A couple of comments, otherwise LGTM. Thanks!

packages/Jinja2/meta.yaml

packages/more-itertools/meta.yaml

packages/pytest/meta.yaml

packages/pyyaml/meta.yaml

rth · 2022-09-04T10:25:19Z

packages/scikit-image/meta.yaml

  version: 0.19.3
+  top-level:
+    - skimage
+    # - doc  # scikit-image has top level `doc` directory... but it is too common name so we don't want to use it as a hint for top-level import.


Reported to scikit-image/scikit-image#6505

pyodide-build/pyodide_build/common.py

pyodide-build/pyodide_build/mkpkg.py

… top-level

ryanking13 · 2022-09-06T06:19:23Z

Thanks for the thorough reviews!

pycryptodomex was added in #2966, but it had an invalid recipe name meta.yml (not meta.yaml) so our build system didn't build that package. I found that in #3006, and I also found that it is not building well, so I disabled it then. So, in other words, pycryptodomex never worked in Pyodide. I would like to remove it from the changelog and the repository for now, so that we don't add it to our next stable release accidentally. Perhaps someone interested can re-add this package.

ryanking13 added 8 commits August 23, 2022 01:02

Parse top level import name from wheel

51dd74e

Print log when parsing top level import fails

d1f2b3a

Find top level files by iterating subdirectories

fb5bd41

Handle top-level python files

c96b739

Fix failing test

fbe9e8b

Add test

3f75142

Merge remote-tracking branch 'upstream/main' into toplevel

bd1272a

Update changelog

fadc9e2

rth reviewed Aug 23, 2022

View reviewed changes

pyodide-build/pyodide_build/buildall.py Outdated Show resolved Hide resolved

pyodide-build/pyodide_build/buildall.py Outdated Show resolved Hide resolved

ryanking13 marked this pull request as draft August 23, 2022 08:30

ryanking13 added 3 commits August 23, 2022 10:09

Parse top-level import during mkpkg

4b988cf

Add top-level keys

462daf7

Handle nested directories

2616e10

Update docs

219be41

ryanking13 commented Aug 24, 2022

View reviewed changes

ryanking13 added 7 commits August 24, 2022 00:47

Add log on assertion error

b3900b2

Use name instead of stem to allow packages containing dots

8d22b4d

Fix requirement name

9f2f391

Allow dot in package name

5a2a835

Fix regex

8bd7190

Merge remote-tracking branch 'upstream/main' into toplevel

2e413fe

Disable pycryptodomex

8640047

ryanking13 marked this pull request as ready for review August 25, 2022 00:39

This was referenced Aug 30, 2022

Cannot install ruamel.yaml: Can't find a pure Python 3 wheel for ruamel-yaml-clib>=0.2.6 #3034

Closed

Fix incorrect package name of ruamel.yaml #3036

Merged

Merge remote-tracking branch 'upstream/main' into toplevel

67e3628

rth approved these changes Sep 4, 2022

View reviewed changes

ryanking13 added 9 commits September 5, 2022 04:19

Merge remote-tracking branch 'upstream/main' into toplevel

582ea1b

Add top-level to coverage package

5cc1768

Remove empty home and licenses

c1d2374

Use top-level key in import test if test/imports not exists

80a0e6d

Remove redundant test/imports keys

9b566c6

Don't add test/imports key automatically in mkpkg command in favor of…

840d8d4

… top-level

Apply suggestions

0749c70

Merge remote-tracking branch 'upstream/main' into toplevel

c968c01

Fix typo

9ca67d5

ryanking13 merged commit 555f782 into pyodide:main Sep 6, 2022

ryanking13 deleted the toplevel branch September 6, 2022 06:20

ryanking13 mentioned this pull request Sep 8, 2022

Add package pycryptodomex #2966

Merged

3 tasks

rth mentioned this pull request Sep 10, 2022

More general unvendoring mechanism #3092

Open

3 tasks

ryanking13 mentioned this pull request Nov 16, 2022

Remove pycryptodomex from the package list #3261

Merged

1 task

ryanking13 mentioned this pull request Oct 11, 2023

Add wheels cli pyodide/pyodide-lock#20

Merged

ryanking13 mentioned this pull request May 25, 2024

Index pip-installed packages pyodide/micropip#109

Draft

ryanking13 mentioned this pull request Feb 15, 2025

micropip.freeze imports field for PyPi dependencies is insufficient pyodide/micropip#207

Open

Uh oh!

Conversation

ryanking13 commented Aug 23, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Checklists

Uh oh!

rth left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

ryanking13 commented Aug 23, 2022

Uh oh!

ryanking13 commented Aug 23, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ofek commented Aug 23, 2022

Uh oh!

ryanking13 left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

hoodmane commented Sep 2, 2022

Uh oh!

rth left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ryanking13 commented Sep 6, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

ryanking13 commented Aug 23, 2022 •

edited

Loading

ryanking13 commented Aug 23, 2022 •

edited

Loading

ryanking13 left a comment •

edited

Loading

rth left a comment •

edited

Loading