GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — bpo-25711: zipimport rewritten in Python by warsaw · Pull Request #4023 · python/cpython

Conversation

@warsaw

@warsaw warsaw commented Oct 17, 2017 •
edited by bedevere-bot
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member

This starts from @serhiy-storchaka 's zipimport-2.patch from bpo-25711. It does not yet work, but it will be easier to comment on and fix as a PR rather than a patch.

https://bugs.python.org/issue25711

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
ncoghlan and AraHaan reacted with hooray emoji
All reactions
  • 🎉 2 reactions
Barry Warsaw
Pure application of Serhiy's zipimport-2.patch
99afd75
@warsaw
warsaw requested a review from a team October 17, 2017 20:43
@the-knights-who-say-ni the-knights-who-say-ni added the CLA signed label Oct 17, 2017
@bedevere-bot bedevere-bot added the awaiting merge label Oct 17, 2017
Barry Warsaw
Add a blurb
35b54ca
ncoghlan
ncoghlan reviewed Oct 18, 2017
View reviewed changes
Comment thread Misc/NEWS.d/next/Core and Builtins/2017-10-17-16-46-49.bpo-25711.gfGaqL.rst
@@ -0,0 +1 @@
zipimport has been rewritten in pure Python.

@ncoghlan ncoghlan Oct 18, 2017

Copy link
Copy Markdown
Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

A separate news entry for the DeprecationWarning -> TypeError change for creating zip importers directly from bytes paths would probably make sense. (Or at least mentioning it in this one)

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
pppery
pppery reviewed Jan 23, 2018
View reviewed changes
Comment thread Lib/zipimport.py

_importing_zlib = False

# Return the zlib.decompress function object, or NULL if zlib couldn't

@pppery pppery Jan 23, 2018

Copy link
Copy Markdown

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

NULL doesn't exist in python. Should be "raises a ZipImportError ..."

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@warsaw

warsaw commented Jan 24, 2018

Copy link
Copy Markdown
Member Author

I wonder if it's really so useful to keep this PR around, especially in light of @Yhg1s promised rewrite?
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@Yhg1s

Yhg1s commented Jan 24, 2018

Copy link
Copy Markdown
Member

It can always be re-opened. I do still plan to open-source my zipimport rewrite, hopefully before PyCon, but it is in C, not Python. If people value having a pure-Python version of zipimport in the standard library, that's a different matter.

(I do recommend not using zipimport.c as a guide for a pure-Python version of zipimport, as this version has done... It has a number of bugs, like how it handles offsets in the central directory and how it closes and then re-opens the ZIP file, which may have changed, without re-reading the central directory. Also, no zip64 support. And as written, zipimport.py (partially) reimplements zipfile, which is a bit of a shame... Although I also have plenty of issues with zipfile :-P)
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@warsaw

warsaw commented Jan 24, 2018

Copy link
Copy Markdown
Member Author

Alright, I'm closing this since I don't think it has a viable future. It was mainly an exercise in trying to convert the original bpo patch into a branch+PR.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@warsaw warsaw closed this Jan 24, 2018
@jaraco jaraco mentioned this pull request Oct 21, 2020
ResourceReader API can't be implemented for zipimporter python/importlib_resources#49
Closed
@warsaw
warsaw deleted the bpo-25711 branch October 12, 2026 23:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ncoghlan ncoghlan ncoghlan left review comments
+1 more reviewer
@pppery pppery pppery left review comments
Reviewers whose approvals may not affect merge requirements

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

awaiting merge

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

6 participants

@warsaw @Yhg1s @ncoghlan @pppery @the-knights-who-say-ni @bedevere-bot
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.