BUG: clongdouble(str) loses precision during string conversion by MyUserNameWasTakenLinux · Pull Request #29746 · numpy/numpy

MyUserNameWasTakenLinux · 2025-09-12T06:14:49Z

This pull request addresses #26701. Converting to a np.clongdouble from a string would result in a loss of precision. This is because the conversion process involves Python's complex type as an intermediate, which has less precision than np.clongdouble. The pull request adds a test which fails on the main branch on linux, x86.

ngoldbaum · 2025-09-12T14:58:56Z

Neat!

Just a head's up - longdouble is a bit of an unloved feature. That said, this seems reasonable to support.

Some suggestions:

I doubt the test you added exercises all the code paths in the code you added. Can you try to make sure your tests cover all the new code you added? Error cases are often poorly tested, for example.
While we're at it, we could fix the casts between the string DTypes and the longdouble dtype - IIRC the casts have the same problem.

I wonder if the linalg test that is failing is relying on the old broken conversion.

Ping @SwayamInSync - you might be interested in this. Swayam has been working on a quad precision DType we're hoping will ultimately allow us to deprecate longdouble support (or at least the float128 alias) in NumPy proper. The fact that longdouble has a subtly different meaning under different architectures is a constant source of bug reports and most users won't need it at all.

WarrenWeckesser · 2025-09-12T15:29:16Z

Not a full review, but here are a couple issues that should be easy to fix:

This will not handle an input such as '0.5j' correctly; it returns 0.5+0j.
It incorrectly accepts an input such as '0.5 plate of shrimp'. It prematurely returns successfully in that case.

WarrenWeckesser · 2025-09-12T17:24:14Z

Again not a full review (sorry), but it looks like the failures are because the new CLONGDOUBLE_setitem() is not correctly handling all its possible inputs. It might be given, for example, a Python float, a Python complex, a NumPy complex64, a NumPy complex128, a NumPy float64, a NumPy longdouble, etc. I don't know the NumPy C API well enough off the top of my head to quickly suggest the minimal set of type checks and conversion functions that would handle all the cases that will be handed to CLONGDOUBLE_setitem().

benburrill · 2025-09-13T04:58:22Z

Probably should allow parentheses (as CPython, and hence np.cdouble does). Python likes to put parentheses in complex reprs, so I think it is important to handle this case at least, even if you're dropping support for some of the more esoteric bits of the CPython parser, like 1-j and 1_000.

The parser can handle parentheses now, though I'm not sure if dropping support for 1-j or 1_000 would be okay.

For the case of 1_000, although it's technically a regression, np.longdouble doesn't support underscores either, so I think it's perfectly reasonable for np.clongdouble not to support them anymore. As for 1-j, CPython says it's for backwards compatibility and may be removed in the future. So in my opinion, probably fine to drop as well. But I do think it would be good to get the opinion of a numpy maintainer.

Supporting it seems easy enough, I would lean towards just keeping it if it's simple and adding a test that will fail if Python ever breaks it.
But if you don't, at least add a test for it raising!

MyUserNameWasTakenLinux · 2025-09-15T02:17:16Z

Currently, I'm relying on Python's complex type to deal with Python types as input to CLONGDOUBLE_setitem(PyObject *op). But this causes a loss in precision when converting from a Python int to np.clongdouble. I tried to use npy_longdouble_from_PyLong to handle the case of converting from a Python int to np.clongdouble, but that change causes test_nep50_weak_integers_with_inexact to fail.

benburrill · 2025-09-15T03:52:38Z

That test specifically excludes longdouble (notice it tests "dDG", but not "g"), which has the same int->string issue, so it would probably fine to just change the test to exclude "G" as well. That's not an ideal solution, I think the test should be rewritten so that if npy_longdouble_from_PyLong is changed in the future to go directly from int->longdouble, the test should either test what it was supposed to test, or xpass or something, but doing the same thing for clongdouble as is done for longdouble seems fine to me.

^{It's kinda funny because IIRC running the numpy tests is what was used to determine how small of a limit they could get away with for integer string conversion, and yet the chosen limit is still is a problem even for the numpy tests...}

MyUserNameWasTakenLinux · 2025-09-18T04:36:40Z

I've added a few tests that cover some of the conversion cases. Please let me know if there is anything you'd like me to change.

Something I don't understand is why x[0] = y from

def test_from_zerodim_array(self):
        x = np.zeros(1, dtype=np.clongdouble)
        py_val = 2.0 + 3.0j
        y = np.array(np.clongdouble(py_val))
        x[0] = y
        assert np.allclose(complex(x[0]), py_val)

doesn't pass through the PyArray_IsZeroDim(op) branch. Instead it goes through PyArray_IsScalar(op, CLongDouble). Is it possible that the PyArray_IsZeroDim branch is never taken?

ngoldbaum · 2025-09-18T18:00:43Z

Is it possible that the PyArray_IsZeroDim branch is never taken?

Maybe? You could try putting a breakpoint inside that branch with a C debugger and see if the breakpoint ever gets hit. Although if the tests don't cover that branch then this exercise wouldn't prove anythig - maybe test coverage is bad? But if it is covered then that might give you more context if you look at the call stack.

I recently added docs for using C debuggers with NumPy's internals, you can try it out: https://numpy.org/devdocs/dev/development_advanced_debugging.html#c-debuggers

seberg

Overall, looks reasonable, I think a few more tests would be good, this type of code is deceptive. But maybe Nathan or Warren have a look through again, then I am happy (with that code duplication removed, it's a lot of lines and it confused me).

seberg · 2025-10-01T11:45:40Z

+        npy_bool has_brackets = NPY_FALSE;
+        start = s;
+
+        while (isspace((unsigned char) *s)) {


Hmmm, we'll lose the ability to parse certain strings in principle that Python parses (e.g. non-breaking space). But clongdouble currently doesn't parse these either. So maybe OK.

seberg · 2025-10-01T11:51:43Z

+                    Py_XDECREF(b);
+                    return temp;
+                }   
+            }


This is a huge repeated block, and not very clear that it is the finalization and will never continue below. Even a goto would seem better, I think just put what is after this into the if instead.

seberg · 2025-10-01T11:58:32Z

+        "not a number",
+        "(2+2j"  # Missing closing brackets
+        "2+5j )"  # Missing opening brackets
+        "3+4k"


I think this could have more tests, e.g. 3j+3, 3+-1j? Also maybe unicode characters for example (but that is different test maybe).

seberg · 2025-10-01T12:06:10Z

Supporting it seems easy enough, I would lean towards just keeping it if it's simple and adding a test that will fail if Python ever breaks it.
But if you don't, at least add a test for it raising!

MyUserNameWasTakenLinux · 2026-01-23T02:42:57Z

Sorry for the long delay, I've removed the code duplication and added some tests for invalid strings.

ngoldbaum

Sorry for letting this sit for so long! I'll also add a milestone while will help us avoid forgetting.

github-actions Bot added the 00 - Bug label Sep 12, 2025

MyUserNameWasTakenLinux marked this pull request as draft September 12, 2025 06:20

benburrill reviewed Sep 13, 2025

View reviewed changes

Comment thread numpy/_core/src/multiarray/arraytypes.c.src Resolved

MyUserNameWasTakenLinux marked this pull request as ready for review September 14, 2025 23:04

seberg reviewed Oct 1, 2025

View reviewed changes

MyUserNameWasTakenLinux force-pushed the clongdouble-precision branch 2 times, most recently from 8e7bfb2 to edd6529 Compare January 22, 2026 18:05

BUG: clongdouble(str) loses precision during string conversion

e4733b9

MyUserNameWasTakenLinux force-pushed the clongdouble-precision branch from edd6529 to e4733b9 Compare January 22, 2026 18:29

ngoldbaum approved these changes Mar 23, 2026

View reviewed changes

ngoldbaum added this to the 2.5.0 Release milestone Mar 23, 2026

charris modified the milestones: 2.5.0 Release, 2.6.0 Release May 24, 2026

Conversation

MyUserNameWasTakenLinux commented Sep 12, 2025

Uh oh!

ngoldbaum commented Sep 12, 2025

Uh oh!

WarrenWeckesser commented Sep 12, 2025

Uh oh!

WarrenWeckesser commented Sep 12, 2025

Uh oh!

Uh oh!

benburrill Sep 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

MyUserNameWasTakenLinux Sep 14, 2025

Choose a reason for hiding this comment

Uh oh!

benburrill Sep 14, 2025

Choose a reason for hiding this comment

Uh oh!

seberg Oct 1, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

MyUserNameWasTakenLinux commented Sep 15, 2025

Uh oh!

benburrill commented Sep 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

MyUserNameWasTakenLinux commented Sep 18, 2025

Uh oh!

ngoldbaum commented Sep 18, 2025

Uh oh!

seberg left a comment

Choose a reason for hiding this comment

Uh oh!

seberg Oct 1, 2025

Choose a reason for hiding this comment

Uh oh!

seberg Oct 1, 2025

Choose a reason for hiding this comment

Uh oh!

seberg Oct 1, 2025

Choose a reason for hiding this comment

Uh oh!

seberg Oct 1, 2025

Choose a reason for hiding this comment

Uh oh!

MyUserNameWasTakenLinux commented Jan 23, 2026

Uh oh!

ngoldbaum left a comment

Choose a reason for hiding this comment

Uh oh!

This branch has not been deployed

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

benburrill Sep 13, 2025 •

edited

Loading

benburrill commented Sep 15, 2025 •

edited

Loading