Skip to content

Fix DrsSymbol and DrsIndexed pickle compatibility with SymPy v1.9+#25

Merged
chenpeizhi merged 2 commits intopep621from
copilot/fix-4a97c3a0-e4fd-44b4-86dc-d920f1373375
Sep 6, 2025
Merged

Fix DrsSymbol and DrsIndexed pickle compatibility with SymPy v1.9+#25
chenpeizhi merged 2 commits intopep621from
copilot/fix-4a97c3a0-e4fd-44b4-86dc-d920f1373375

Conversation

Copy link
Contributor

Copilot AI commented Sep 6, 2025

Problem

The test_pickle_within_drs test was failing with SymPy v1.9+ due to pickle incompatibility:

TypeError: DrsSymbol.__new__() missing 1 required positional argument: 'name'

This error occurred when trying to unpickle DrsSymbol and DrsIndexed objects within drudge scripts.

Root Cause

SymPy v1.9+ introduced the __getnewargs_ex__() method to its Symbol class, which takes precedence over the existing __getnewargs__() method during pickle deserialization. The issue manifested in two ways:

  1. Argument Mismatch: SymPy's __getnewargs_ex__() returns (('symbol_name',), {}) providing only one argument, but DrsSymbol.__new__(cls, drudge, name) expects two arguments.

  2. Missing Initialization: When __getnewargs_ex__() provides sufficient arguments for __new__, Python skips calling __setstate__(), leaving the _orig attribute uninitialized and causing AttributeError: 'DrsIndexed' object has no attribute '_orig'.

Solution

This PR implements a two-part fix:

1. Override __getnewargs_ex__() Methods

Added __getnewargs_ex__() methods to both DrsSymbol and DrsIndexed classes to provide the correct arguments for their respective __new__() methods:

def __getnewargs_ex__(self):
    """Get the arguments and keyword arguments for __new__.
    
    This method takes precedence over __getnewargs__ in Python 3.6+
    and is used by SymPy v1.9+.
    """
    return (None, self.name), {}  # For DrsSymbol
    # or 
    return ((None, self.base) + self.indices), {}  # For DrsIndexed

2. Ensure __setstate__() is Called

Modified __getstate__() methods to return {} instead of None, ensuring that __setstate__() is always called during unpickling to properly initialize the _orig attribute and other instance variables.

Testing

  • ✅ The originally failing test_pickle_within_drs now passes
  • ✅ All existing pickle-related tests continue to pass
  • ✅ No regressions detected in the full test suite
  • ✅ Maintains backward compatibility while ensuring forward compatibility with SymPy v1.9+

Files Changed

  • drudge/drs.py: Added __getnewargs_ex__() methods and modified __getstate__() methods for both DrsSymbol and DrsIndexed classes

This fix follows Python's standard pickle protocol and should work reliably across different Python and SymPy versions.


💬 Share your feedback on Copilot coding agent for the chance to win a $200 gift card! Click here to start the survey.

@coveralls
Copy link

coveralls commented Sep 6, 2025

Pull Request Test Coverage Report for Build 17519284021

Details

  • 6 of 6 (100.0%) changed or added relevant lines in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.2%) to 87.996%

Totals Coverage Status
Change from base Build 17518696745: 0.2%
Covered Lines: 3790
Relevant Lines: 4307

💛 - Coveralls

Co-authored-by: chenpeizhi <8114085+chenpeizhi@users.noreply.github.com>
Copilot AI changed the title [WIP] test_pickle_within_drs in tests/free_algebra_test.py failed with error message "TypeError: DrsSymbol.__new__() missing 1 required positional argument: 'name'". This is probably due to the new __getnewargs_ex__ method in SymPy v1.9. Follow the fol... Fix DrsSymbol and DrsIndexed pickle compatibility with SymPy v1.9+ Sep 6, 2025
Copilot AI requested a review from chenpeizhi September 6, 2025 20:38
@chenpeizhi chenpeizhi marked this pull request as ready for review September 6, 2025 20:43
@chenpeizhi chenpeizhi merged commit a28d112 into pep621 Sep 6, 2025
5 of 6 checks passed
@chenpeizhi chenpeizhi deleted the copilot/fix-4a97c3a0-e4fd-44b4-86dc-d920f1373375 branch September 17, 2025 01:48
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