Skip to content

Stop importing yt-dlp, bs4 and lxml when importing zim or inputs - #403

Open
pawank925 wants to merge 1 commit into
openzim:mainfrom
pawank925:issue-244-cut-zim-import-edges
Open

pawank925 wants to merge 1 commit into
openzim:mainfrom
pawank925:issue-244-cut-zim-import-edges

Conversation

@pawank925

Copy link
Copy Markdown
Contributor

Step 1 of the approach in #244. No dependency changed, no name moved, nothing a scraper calls behaves differently.

Measured

sys.modules of a fresh interpreter with every dependency installed:

import main this branch
zimscraperlib.zim 19 of the 23 runtime deps 16
zimscraperlib.inputs 5 1 (beartype)

Gone from the zim graph: yt_dlp, bs4, lxml.

Correction to what I quoted in #244: I said 13, from a static AST walk that missed transitive imports. The runtime number is 19 of 23, so the graph is worse than stated, not better.

Changes

  • inputs.py: stream_file imported in the http branch of handle_user_provided_file, so a local path no longer loads the download stack.
  • zim/items.py: stream_file imported in URLItem.download_for_size and URLItem.__init__.
  • zim/providers.py: get_retry_adapter in URLProvider.__init__, stream_file in URLProvider.get_size_of.
  • zim/filesystem.py: find_title_in_file in FileItem.get_title.
  • tests/inputs/test_inputs.py: the three monkeypatch.setattr calls now patch zimscraperlib.download — the module the name is looked up on at call time.
  • tests/test_import_boundaries.py: new. Hides yt-dlp (or bs4+lxml) from a child interpreter with a sys.meta_path finder and asserts zimscraperlib.zim / zimscraperlib.inputs import anyway, then asserts that hiding yt-dlp does break zimscraperlib.download — without that self-check the first assertions could pass vacuously.
  • pyproject.toml: PLC0415 ignored, with the reason. The rule forbids precisely the pattern this PR introduces and step 2 spreads, so either the config says so or 6 # noqas do.

Two edges I did not cut, and why

zim/metadata.py keeps is_valid_image and is_valid_iso_639_3 at module level, and unique_values stays in inputs.

  • Moving those two into get_cleaned_value changes no number: metadata.py builds DEFAULT_DEV_ZIM_METADATA while it is being imported, and the illustration and language entries in it are validated through pillow and Babel at that moment. So the modules load at import zimscraperlib.zim either way, and the edit would be churn. Cutting that edge for real means not validating that constant at import time, which is a behavior change rather than an import move — a step 2 decision. For the size of the prize I measured it with a throwaway patch skipping the construction: zim drops from 16 to 11, and to 8 (beartype, idna, libzim, pymupdf-lite, python-magic, regex, requests, urllib3) once creator.py's PIL.Image and i18n imports get the same call-level treatment. That patch is not in this PR.
  • unique_values did not need to leave inputs. Once stream_file is call-level there, inputs pulls nothing but the package itself, so metadata -> inputs costs no dependency, and moving it would be a public name move for nothing.

Tests

  • pytest --runslow --runinstalled: 1325 passed, 37 failed, 33 skipped. The same 37 fail on a clean main here (no ffmpeg, no gifsicle; exit 127), so nothing moved in either direction.
  • ruff 0.17.3 check and format --check, pyright 1.1.414: clean.

Move stream_file, get_retry_adapter and find_title_in_file into the
functions that call them, so importing a family no longer loads another
family's dependencies. No dependency changed and no API moved.
@pawank925

pawank925 commented Nov 13, 2026 •

Copy link
Copy Markdown
Contributor Author

Step 1 only. #244 stays open; step 2 (the extras themselves) still needs a decision from @benoit74 since it shrinks the default install.

@codecov

codecov Bot commented Nov 13, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 100.00%. Comparing base (657fe48) to head (9ff4fbb).

Additional details and impacted files
@@            Coverage Diff            @@
##              main      #403   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           42        42           
  Lines         2952      2954    +2     
  Branches       452       452           
=========================================
+ Hits          2952      2954    +2     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@pawank925

Copy link
Copy Markdown
Contributor Author

@benoit74

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant