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

Pre-release job is failing against master #1108

Closed
jacobdr opened this issue May 8, 2022 · 1 comment · Fixed by #1110
Closed

Pre-release job is failing against master #1108

jacobdr opened this issue May 8, 2022 · 1 comment · Fixed by #1110

Comments

@jacobdr
Copy link
Contributor

jacobdr commented May 8, 2022

most recent failing job as of this writing May 7

Trying to narrow differences... since mmap is coming from the standard lib I suspected a change in Python versions and how repr(mmap_obj) or str(mmap_obj) is displayed... Its possible, though still smells to me so needs a sanity check.

As evidence, though, comparing against a prior (chose #1096 as the most recent unmerged green build I saw....)

Python version comparison

Final note: the only pre-release job that passes is the one that does not have the PRE_PIP_FLAGS environment variable set....

Originally posted by @jacobdr in #1105 (comment)

@effigies
Copy link
Member

effigies commented May 9, 2022

Manually bisecting the pre-release wheels of numpy (https://pypi.anaconda.org/scipy-wheels-nightly/simple/numpy/) I can say that this arose between numpy-1.23.0.dev0+1065.gf996a2b62 and numpy-1.23.0.dev0+1087.g0eaa40db3.

Looking at numpy/numpy@f996a2b...0eaa40d, I see they merged numpy/numpy#21324 in that interval, which seems likely to be the cause. It does not look like it actually performs a copy, so that's probably okay. It looks like the reason the test was written this way:

NetCDF files, when opened read-only, return arrays that refer
directly to memory-mapped data on disk:

    >>> data = time[:]
    >>> data.base.base  # doctest: +ELLIPSIS
    <mmap.mmap ...>

is because the object is not actually a numpy.memmap but a custom mmap.mmap-backed ndarray.

It looks like scipy didn't hit this because they dropped this test (although not the surrounding text) in scipy/scipy@6d40ad4 as part of scipy/scipy#14832.

Not quite sure where that leaves us. The easy thing is to follow Scipy's lead and drop the test, though I'd then go ahead and drop the text that indicates that mmap should appear. I don't know if the better solution would be to find an alternative test to ensure that the mmap functionality isn't damaged by future releases, and ensure it gets upstreamed into scipy.

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 a pull request may close this issue.

2 participants