Skip to content

Commit 9aff82c

Browse files
author
Release Manager
committed
sagemathgh-41206: Fix fragile doctest sorting in multi_polynomial_ideal ### Description This PR fixes a fragile doctest failure reported in sagemath#41193. **The Issue:** The failing test in `src/sage/rings/polynomial/multi_polynomial_ideal.py` used `sorted(..., key=str)` on a list of dictionaries. In Python, the string representation of a dictionary depends on the insertion order of its keys. Since the upstream library `msolve` likely changed its internal insertion order in newer versions, `key=str` produced a different sort order than expected, causing the test to fail despite the mathematical results being correct. **The Fix:** I updated the test command to replace the unstable sort key with a robust one. This sorts the dictionary items before stringifying, ensuring the sorting relies on the dictionary *contents* rather than their internal memory layout. ```python # Old (fragile) key=str # New (robust) key=lambda d: str(sorted(d.items())) ``` **Verification** I verified that this change makes the sorting immune to dictionary insertion order. **Proof of Fix**: Running the following script demonstrates why `key=str` failed and why the new key succeeds: ```python dict_A = {'x': 100, 'y': 500} dict_B = {'y': 500, 'x': 100} # ---Old Behavior (key=str)--- # str(dict_A) is "{'x': 100, 'y': 500}" # str(dict_B) is "{'y': 500, 'x': 100}" print(f"Old method equal? {str(dict_A) == str(dict_B)}") # Result: False (This caused the test failure) # --- New Behavior (The Fix) --- my_key = lambda d: str(sorted(d.items())) print(f"New method equal? {my_key(dict_A) == my_key(dict_B)}") # Result: True (The sort is now stable) ``` This ensures the doctest passes reliably regardless of the upstream library's behavior. ### **Issue** Fixes sagemath#41193 Relates to sagemath#41192 ### 📝 Checklist - [x] The title is concise and informative. - [x] The description explains in detail what this PR is about. - [x] I have linked a relevant issue or discussion. - [x] I have created tests covering the changes. - [ ] I have updated the documentation and checked the documentation preview. ### ⌛ Dependencies None. URL: sagemath#41206 Reported by: Hetarth Jodha Reviewer(s):
2 parents ad3fd44 + cb31e81 commit 9aff82c

File tree

1 file changed

+1
-1
lines changed

1 file changed

+1
-1
lines changed

src/sage/rings/polynomial/multi_polynomial_ideal.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2617,7 +2617,7 @@ def variety(self, ring=None, *, algorithm='triangular_decomposition', proof=True
26172617
sage: I = Ideal([x^2 - 1, y^2 - 1]) # needs sage.rings.finite_rings
26182618
sage: sorted(I.variety(algorithm='msolve', # optional - msolve, needs sage.rings.finite_rings
26192619
....: proof=False),
2620-
....: key=str)
2620+
....: key=lambda d: str(sorted(d.items()))
26212621
[{y: 1, x: 1},
26222622
{y: 1, x: 536870908},
26232623
{y: 536870908, x: 1},

0 commit comments

Comments
 (0)