Skip to content

Make patch_submodule robust to __builtins__ being a module (PyPy / __main__)#8310

Open
vineethsaivs wants to merge 1 commit into
huggingface:mainfrom
vineethsaivs:fix/patch-submodule-builtins-dict
Open

Make patch_submodule robust to __builtins__ being a module (PyPy / __main__)#8310
vineethsaivs wants to merge 1 commit into
huggingface:mainfrom
vineethsaivs:fix/patch-submodule-builtins-dict

Conversation

@vineethsaivs

Copy link
Copy Markdown

Closes #7636.

Problem

patch_submodule reads the builtins through globals()["__builtins__"]:

elif target_attr in globals()["__builtins__"]:  # if it's a builtin like "open"
    self.original[target_attr] = globals()["__builtins__"][target_attr]

The value of a module's __builtins__ global is a CPython implementation detail: it is the builtins module (not its dict) in the __main__ module and under PyPy, and only usually a dict in imported modules (docs). When it is the module, target_attr in globals()["__builtins__"] raises:

TypeError: argument of type 'module' is not iterable

so patching a builtin such as open fails. This is what #7636 hit running datasets streaming under PyPy.

Fix

Use builtins.__dict__ directly, which is always the builtins mapping regardless of the runtime. In the common CPython case (an imported module whose __builtins__ already is builtins.__dict__) the behavior is unchanged; it just also works under __main__/PyPy.

Test

test_patch_submodule_builtin_when_builtins_is_module sets datasets.utils.patching.__builtins__ to the builtins module (simulating the PyPy / __main__ runtime) and patches open. It raises the TypeError above on the current code and passes with this change; the existing patching tests are unaffected (full tests/test_patching.py is green).

patch_submodule read the builtins via globals()["__builtins__"]. The value
of a module's __builtins__ global is a CPython implementation detail: it is
the builtins module (not its dict) in the __main__ module and under PyPy,
and only usually a dict in imported modules. When it is the module,
'target_attr in globals()["__builtins__"]' raises
'TypeError: argument of type \'module\' is not iterable', so patching a
builtin such as open fails (e.g. when running datasets streaming under
PyPy, as reported in huggingface#7636).

Use builtins.__dict__ directly, which is always the builtins mapping
regardless of the runtime. Add a regression test that simulates the module
case.

Closes huggingface#7636
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.

"open" in globals()["__builtins__"], an error occurs: "TypeError: argument of type 'module' is not iterable"

1 participant