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

ENH correctly restore default_factory of a defaultdict #433

Merged
merged 7 commits into from
Aug 8, 2024
Merged

ENH correctly restore default_factory of a defaultdict #433

Changes from 1 commit
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
c5336ed
ENH correctly restore default_factory of a defaultdict
adrinjalali Jul 11, 2024
ca649ae
MNT add changelog for new version and bump version
adrinjalali Jul 11, 2024
1bb275c
add OrderedDict trusted
adrinjalali Jul 12, 2024
4fe05f1
Merge remote-tracking branch 'upstream/main' into default_dict
adrinjalali Jul 22, 2024
cfab777
rename builtin type names to container type names
adrinjalali Jul 22, 2024
f0a0e05
add test for OrderedDict
adrinjalali Jul 22, 2024
d118520
test type as well
adrinjalali Aug 8, 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
9 changes: 8 additions & 1 deletion skops/io/tests/test_persist.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
import operator
import sys
import warnings
from collections import Counter, defaultdict
from collections import Counter, OrderedDict, defaultdict
from functools import partial, wraps
from pathlib import Path
from zipfile import ZIP_DEFLATED, ZipFile
Expand Down Expand Up @@ -1092,3 +1092,10 @@ def test_defaultdict():
obj_loaded = loads(dumps(obj))
assert obj_loaded == obj
assert obj_loaded.default_factory == obj.default_factory
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should a test for OrderedDict be added too?



@pytest.mark.parametrize("cls", [dict, OrderedDict])
def test_dictionary(cls):
obj = cls({1: 5, 6: 3, 2: 4})
loaded_obj = loads(dumps(obj))
assert obj == loaded_obj
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should the type also be checked? E.g. this passes:

assert {2: 20, 1: 10} == OrderedDict([(2, 20), (1, 10)])
assert {1: 10, 2: 20} == OrderedDict([(2, 20), (1, 10)])

Copy link
Collaborator

Choose a reason for hiding this comment

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

ping @adrinjalali

Copy link
Member Author

Choose a reason for hiding this comment

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

oops, sorry. Added now @BenjaminBossan

Loading