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.

Sign up for GitHub

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

Jump to bottom

Fix pickle/copy support for the missing singleton #2029

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Fix pickle/copy support for the missing singleton #2029

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
5289308
Fix pickle/copy support for the `missing` singleton
mattclay Oct 1, 2024
0a87e81
[pre-commit.ci lite] apply automatic fixes
pre-commit-ci-lite[bot] Oct 1, 2024
d3dd9f2
Define `missing` using an actual class
mattclay Oct 2, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGES.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@ Version 3.2.0
Unreleased

- Drop support for Python 3.7.
- Fix pickle/copy support for the ``missing`` singleton.
:pr:`2029`
- Use modern packaging metadata with ``pyproject.toml`` instead of ``setup.cfg``.
:pr:`1793`
- Use ``flit_core`` instead of ``setuptools`` as build backend.
Expand Down
6 changes: 5 additions & 1 deletion src/jinja2/utils.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,11 @@
F = t.TypeVar("F", bound=t.Callable[..., t.Any])

# special singleton representing missing values for the runtime
missing: t.Any = type("MissingType", (), {"__repr__": lambda x: "missing"})()
missing: t.Any = type(
"MissingType",
(),
{"__repr__": lambda x: "missing", "__reduce__": lambda x: "missing"},
)()
Copy link
Author

@mattclay mattclay Oct 1, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

I used this form to minimize changes, but it could be replaced with an actual class definition if desired.

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, that makes more sense. I don't see a reason a "singleton" was done this way, it's enough to call the class _MissingType to indicate it's not public.

Copy link

Choose a reason for hiding this comment

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

Always the challenge when interacting with unfamiliar projects/maintainers around finding the balance of "minimize changes" vs "if you were building this today, it would look very different" 😆


internal_code: t.MutableSet[CodeType] = set()

Expand Down
12 changes: 12 additions & 0 deletions tests/test_utils.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import copy
import pickle
import random
from collections import deque
Expand Down Expand Up @@ -183,3 +184,14 @@ def test_consume():
consume(x)
with pytest.raises(StopIteration):
next(x)


@pytest.mark.parametrize("protocol", range(pickle.HIGHEST_PROTOCOL + 1))
def test_pickle_missing(protocol: int) -> None:
"""Test that missing can be pickled while remaining a singleton."""
assert pickle.loads(pickle.dumps(missing, protocol)) is missing


def test_copy_missing() -> None:
"""Test that missing can be copied while remaining a singleton."""
assert copy.copy(missing) is missing