-
-
Notifications
You must be signed in to change notification settings - Fork 127
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
TST: a failing doctest on MacOS + numpy 2 #1146
Comments
I suspect that is an unfortunate coincidence in the original numbers, that |
if we do just that, yes, it will break, because the number of
I tried it, but it doesn't seem to have the desired effect (I'm guessing the embedded table might not be composed of numpy arrays at all). In case we circle back to this idea later however, note that we should use the context manager |
Unfortunate, but I also noticed it seemed to have no effect on numpy scalars like
I thought of that as well, but isn't every section running in its own environment anyway – at least each Last way out I can think of would be to change the spectrum parameters slightly so they don't round off to fewer digits, but that could end up in wild guessing. Still, hiding the actual table values kind of beats the whole point of the documentation in my view; I'd rather |
Oh but I'm not guessing here: I actually tried your suggestion and found that it broke 3 other tests even with numpy 1.x |
Oh, and I thought you found no effect with |
So I see two ways this can go down:
I wouldn't even try to go with 2) unless you greenlight it, as I'm really unsure how long this would take and if it would even work out in the end. |
Looking here specutils/specutils/fitting/fitmodels.py Line 61 in d16d91a
I think it is basically an existing 👎 on blind ellipsis because it makes the example very opaque. |
It is possible |
I confirm that we didn't need to do anything for modeling and I concur to your conclusion.
It is not. |
Looking at the log again
Would ellipsis work partially like this?
|
I'm not saying it doesn't, but I couldn't figure it out after a 30min attempt 🤷🏻♂️ |
The varying line length is indeed surprising... and what does that have to do with numpy rounding? 🤯 |
|
Well it's caused by a difference in rounding: we get a different number of decimals depending on the platform and in turn, the
This is just my current conjecture but what I think is happening is that numpy 2.0 made some performance improvements specifically for macOS arm64 (using the |
Note that the last sentence before this doctest contains at least one typo ( |
As I reproduce how this is created in A somewhat hackish way to resolve this in |
I still get pinged by Actions about this once a week or so. |
#1159 isn't particularly elegant but avoids the failure by printing the three parameters individually, rather than printing the table with all three at once. Any objections? |
There were a number of intentional changes for MacOS arm64 in numpy 2.0, which was released 3 days ago.
It seems that as a side effect of the performance optimizations conducted a table printed in
docs/fitting.rst
changed;# doctest: +FLOAT_CMP
helpfully allows to ignore discrepancies in lower digits, but it doesn't protect against what's happening here: the number of digits printed changed.See example logs.
I tried to make this doctest more portable using the
+ELLIPSIS
directive and ended up with the following patchI could confirm it fixes the test on macos arm64, but it's inconvenient that I just ended up hiding the whole table; unfortunately my attempts to only hide parts of it did not succeed.
Would this patch be accepted as is ?
The text was updated successfully, but these errors were encountered: