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

TST: Handle new numpy 2.0.dev string representation #210

Open
1 task
pllim opened this issue Jul 30, 2023 · 8 comments
Open
1 task

TST: Handle new numpy 2.0.dev string representation #210

pllim opened this issue Jul 30, 2023 · 8 comments

Comments

@pllim
Copy link
Contributor

pllim commented Jul 30, 2023

Example log: https://github.com/scientific-python/pytest-doctestplus/actions/runs/5704149430/job/15457469475

____________________________ [doctest] module2.foo _____________________________
EXAMPLE LOCATION UNKNOWN, not showing all tests of that example
??? >>> foo(1, 2)
Expected:
    3.0
Got:
    np.float64(3.0)

module2.py:None: DocTestFailure
@bsipocz
Copy link
Member

bsipocz commented Aug 1, 2023

Shall we do the easy thing with the legacy printoption, or you rather suggest to make a new directive that does this flexibly? (I suppose we need to do the printoption either way to fix the CI, but for downstream, it maybe useful to have a directive? Or should they all just update the reprs?

cc @seberg

@bsipocz bsipocz added this to the 1.0.0 milestone Aug 1, 2023
@pllim
Copy link
Contributor Author

pllim commented Aug 1, 2023

I have mixed feeling about the new printout. This extra np.float64(...) really messes up the WCS print over at astropy.wcs. I wish there is a way to opt out permanently (not just temporarily).

@drammock
Copy link

drammock commented Aug 1, 2023

@pllim IDK how astropy.wcs works, but would it work to redefine the __print__ and/or __repr__ definitions to cast to python numeric types before printing (leaving the underlying object(s) as a numpy dtype)?

@pllim
Copy link
Contributor Author

pllim commented Aug 1, 2023

Re: WCS -- Maybe but it is just a small portion of things that would be similarly affected. For us downstream that don't see the point of extra stuff in printouts, this seems like busywork that we don't have time for. I think one of the WCS printout code is at https://github.com/astropy/astropy/blob/8ab8208a4df5c636054f07c763006324d245ddbd/astropy/wcs/wcs.py#L3116

@pllim
Copy link
Contributor Author

pllim commented Aug 1, 2023

But that said, I'll check out what Marten has worked out at astropy/astropy#15065 though so far he didn't touch WCS.

@seberg
Copy link
Contributor

seberg commented Aug 1, 2023

There is a !r there, if you know you got a numerical value, using !s should be fine? I don't care about making it more permanent, although i would care a bit about making printoption saner (e.g. context safe).

@pllim
Copy link
Contributor Author

pllim commented Aug 20, 2023

I thought we ignored it but it came back last night (https://github.com/scientific-python/pytest-doctestplus/actions/runs/5915248667/job/16041319959). So I took the liberty to relax the numpy-dev version check directly on main at 91104d8 since it is Sunday morning and I don't have much time. FYI.

@bsipocz
Copy link
Member

bsipocz commented Aug 22, 2023

@pllim - I don't think it's necessary to push hot fixes like this, as 1) the failing cron only an effect in our internal testing 2) we still don't provide a solution for downstream.

Besides, leaving an explicit dev version would be helpful while one is in the middle of the dev cycle.

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

No branches or pull requests

4 participants