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.

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

fix deepcopy & pickle for unsaved model instances & empty image fields #324

Merged
merged 2 commits into from
Nov 6, 2023

Conversation

syphar
Copy link

@syphar syphar commented Oct 5, 2023

While trying to upgrade to v6 of this library we stumbled onto what we think is a bug.

When trying to deepcopy, or pickle an model instance that:

  • either is unsaved
  • or the imagefield is optional and None

Then the __getstate__ implementation will break since the descriptor doesn't include the variations.

This was broken since #265, and released in 5.3.3 (I think).

I understand if this would be not seen as a bug, or not being merged because this library is unmaintained, then we would probably fork it and use the fork until we fully migrated.

Copy link
Owner

@codingjoe codingjoe left a comment

Choose a reason for hiding this comment

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

Hi @syphar, I fixed a linter issue upstream. Once you include the latest changes and drop the obsolete import, the test suite will run. Cheers!

tests/test_models.py Outdated Show resolved Hide resolved
@SchrodingersGat
Copy link

I came here to report this exact issue after scratching my head for over a year :)

If this can be accepted and released, that would be amazing!

@syphar
Copy link
Author

syphar commented Oct 19, 2023

@codingjoe I forgot about your comment to finish this up, sorry for the delay.

I removed the unused import & rebased on master.

@syphar syphar requested a review from codingjoe October 19, 2023 05:01
Copy link
Owner

@codingjoe codingjoe left a comment

Choose a reason for hiding this comment

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

More, broken things in the CI suite. Anyhow, I tested it locally. I will release this change.

@codingjoe codingjoe merged commit bc9af2a into codingjoe:master Nov 6, 2023
7 of 19 checks passed
@SchrodingersGat
Copy link

@codingjoe thanks for the quick fix, much appreciated :)

@syphar syphar deleted the fix-deepcopy-and-pickle branch November 7, 2023 07:00
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.

3 participants