Skip to content

ENH: Allow object and subarray dtypes in fromiter#20993

Merged
mattip merged 7 commits intonumpy:mainfrom
seberg:fixup-fromiter
May 10, 2022
Merged

ENH: Allow object and subarray dtypes in fromiter#20993
mattip merged 7 commits intonumpy:mainfrom
seberg:fixup-fromiter

Conversation

@seberg
Copy link
Member

@seberg seberg commented Feb 3, 2022

Allows dtypes with objects (and subarray) in fromiter. There was
never really a good reason for rejecting them.

Otherwise does some maintanence (e.g. the array is contiguous, so
no need to do complicated index calculations), improve the error
message, and add tests for those things.

Closes gh-4791, gh-15789

@seberg seberg changed the title ENH: Allow object and subarray dtypes in loadtxt ENH: Allow object and subarray dtypes in fromiter Feb 3, 2022
@seberg seberg force-pushed the fixup-fromiter branch 2 times, most recently from 4e0d5b8 to b93c5f6 Compare February 3, 2022 22:15
Allows dtypes with objects (and subarray) in fromiter.  There was
never really a good reason for rejecting them.

Otherwise does some maintanence (e.g. the array is contiguous, so
no need to do complicated index calculations), improve the error
message, and add tests for those things.

Closes numpygh-4791, numpygh-15789
@BvB93 BvB93 linked an issue Feb 4, 2022 that may be closed by this pull request
@BvB93
Copy link
Member

BvB93 commented Feb 11, 2022

Oooh neat, does the mean it's now also possible to create >1D arrays with the likes of, e.g., dtype=("f8", (10,))?

Also, this feels worthy of a release note in my opinion (and maybe an extra example in the docstring?).

@seberg
Copy link
Member Author

seberg commented Feb 11, 2022

Oooh neat, does the mean it's now also possible to create >1D arrays with the likes of, e.g., dtype=("f8", (10,))?

If you must, yes... but it does that via the subarray dtype, so it might be subtly different from what you expect.

@seberg
Copy link
Member Author

seberg commented Feb 11, 2022

Added something, unfortunately, also triggered an unrelated bug on the way. (the example works, the bug is in some error path)

@seberg
Copy link
Member Author

seberg commented Feb 11, 2022

No, just me being stupid, wrote < -1 instead of < 0 and we had not a single test for the failure path (I am surprised code coverage was not bothered by that?)

The iteration bailing out on NULL seems to hide bugs in most cases, so the test suite didn't find it.

Seems I forgot to commit this one...
@mattip
Copy link
Member

mattip commented May 10, 2022

This all looks good, but was never reviewed. @seberg is there a reason not to put this in?

@seberg
Copy link
Member Author

seberg commented May 10, 2022

No, I think review just stalled and should be good. We probably have to apply your stride fix here as well (probably just meaning to remove/modify RELAXED_STRIDES_DEBUG ifdef part).

@mattip mattip merged commit 369a677 into numpy:main May 10, 2022
@mattip
Copy link
Member

mattip commented May 10, 2022

Let's merge this then I will rebase #21477 on top

@seberg
Copy link
Member Author

seberg commented May 10, 2022

Thanks!

@seberg seberg deleted the fixup-fromiter branch May 10, 2022 09:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

np.fromiter crashes if NPY_RELAXED_STRIDES_DEBUG is set Let fromiter handle object arrays

3 participants