Skip to content

Commit

Permalink
Whitelist str for skipping read-only attributes
Browse files Browse the repository at this point in the history
* Fix #478 by whitelisting str for skipping read-only attrs

* Add changelog entry for str errors

* Move new fix from 3.0.3 to 3.0.4
  • Loading branch information
Theelx committed Mar 11, 2024
1 parent 51580d9 commit 423faa8
Show file tree
Hide file tree
Showing 2 changed files with 9 additions and 3 deletions.
8 changes: 7 additions & 1 deletion CHANGES.rst
@@ -1,11 +1,17 @@
v3.0.4
======
* Fixed an issue with django.SafeString and other classes inheriting from
str having read-only attribute errors (#478) (+481)

v3.0.3
======
* Compatibilty with Pandas and Cython 3.0 was added. (#460) (+477)
* Fixed a bug where pickling some built-in classes (e.g. zoneinfo)
could return a ``None`` module. (#447)
* Fixed a bug where unpickling a missing class would return a different object
instead of ``None``. (+471)
* Fixed the handling of missing classes when setting ``on_missing`` to ``warn`` or ``error``. (+471)
* Fixed the handling of missing classes when setting ``on_missing`` to ``warn``
or ``error``. (+471)
* The test suite was made compatible with Python 3.12.
* The tox configuration was updated to generate code coverage reports.
* The suite now uses ``ruff`` to validate python code.
Expand Down
4 changes: 2 additions & 2 deletions jsonpickle/unpickler.py
Expand Up @@ -626,11 +626,11 @@ def _restore_from_dict(self, obj, instance, ignorereserved=True):
# i think this is the only way to set frozen dataclass attrs
object.__setattr__(instance, k, value)
except AttributeError as e:
# some objects may raise this for read-only attributes (#422)
# some objects may raise this for read-only attributes (#422) (#478)
if (
hasattr(instance, "__slots__")
and not len(instance.__slots__)
and issubclass(instance.__class__, int)
and issubclass(instance.__class__, (int, str))
):
continue
raise e
Expand Down

3 comments on commit 423faa8

@Theelx
Copy link
Contributor Author

@Theelx Theelx commented on 423faa8 Mar 11, 2024

Choose a reason for hiding this comment

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

@davvid Do you by any chance know what's causing the ruff tests to fail for this?

@davvid
Copy link
Member

@davvid davvid commented on 423faa8 Mar 12, 2024

Choose a reason for hiding this comment

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

I'll prep an MR shortly. There's a breaking change in pytest-ruff with newer versions of pytest.

businho/pytest-ruff#15

pytest-dev/pytest#12069

I originally worked around it by excluding specifically pytest-ruff 0.3.1 but it looks like they just released 0.3.2 and the issue remains.

I'll pin it down to pytest-ruff < 0.3 shortly. That gets it to pass make tox locally here.

@davvid
Copy link
Member

@davvid davvid commented on 423faa8 Mar 12, 2024

Choose a reason for hiding this comment

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

It should also be noted that as soon as we upgrade to ruff 0.3 the format does change (make format results in edits).

We can defer that until later when we know that pluggy + pytest + ruff are all in good shape.

Please sign in to comment.