8000 ENH: Add support for file like objects to np.core.records.fromfile by sidhant007 · Pull Request #16675 · numpy/numpy · GitHub
[go: up one dir, main page]
Skip to content

ENH: Add support for file like objects to np.core.records.fromfile #16675

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 5 commits into from
Aug 13, 2020
Merged

ENH: Add support for file like objects to np.core.records.fromfile #16675

merged 5 commits into from
Aug 13, 2020

Conversation

sidhant007
Copy link
Contributor
@sidhant007 sidhant007 commented Jun 24, 2020 •
edited
Loading

This PR closes #2504

Now np.core.records.fromfile will support file like objects following the implementation advice mentioned here

Example:

from io import BytesIO
import numpy as np
x = np.array([1, 2, 3])
bytes_array = BytesIO()
bytes_array.write(x.tobytes())
bytes_array.seek(0)
y = np.core.records.fromfile(bytes_array, formats=np.uint, shape=3)

@sidhant007 sidhant007 changed the title ENH: Add file like support to np.core.records.fromfile ENH: Add support for file like objects to np.core.records.fromfile Jun 24, 2020
rossbar
rossbar reviewed Jun 26, 2020
View reviewed changes
Copy link
Contributor
@rossbar rossbar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is the addition of the isfilelikeobj necessary? The comment from 2504 indicates that the current implementation of isfileobj can be replaced with the hasattr check. Did you try this approach?

eric-wieser
eric-wieser reviewed Jun 26, 2020
View reviewed changes
eric-wieser
eric-wieser reviewed Jun 26, 2020
View reviewed changes
eric-wieser
eric-wieser reviewed Jun 26, 2020
View reviewed changes
anirudh2290
anirudh2290 reviewed Jun 26, 2020
View reviewed changes
@sidhant007 sidhant007 force-pushed the recarray-fromfile-fix branch from cf1f5bc to ccb84ff Compare June 29, 2020 03:10
rossbar
rossbar reviewed Jul 2, 2020
View reviewed changes
@sidhant007 sidhant007 requested a review from rossbar July 4, 2020 09:59
rossbar
rossbar approved these changes Jul 14, 2020
View reviewed changes
Copy link
Contributor
@rossbar rossbar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for sticking with this through that conversation about compat/isfileobj, and thanks @anirudh2290 for helping to scope this out!

This LGTM. My only remaining comments would be

  1. Consider adding a comment similar to that added on L914-915 to the corresponding place in the array function (~L1003)
  2. I also like @eric-wieser 's suggestion of using isinstance(f, (io.RawIOBase, io.BufferedIOBase)), which I think could be used in place of the hasattr check and is a little easier to understand.

@mattip
Copy link
Member
mattip commented Aug 2, 2020

@sidhant007 besides the two small fixes, this needs a release note since we are enhancing behaviour.

@rossbar rossbar added the 56 - Needs Release Note. Needs an entry in doc/release/upcoming_changes label Aug 3, 2020
@mattip mattip force-pushed the recarray-fromfile-fix branch from 32f71e8 to c2727d0 Compare August 10, 2020 07:44
@mattip mattip removed the 56 - Needs Release Note. Needs an entry in doc/release/upcoming_changes label Aug 10, 2020
@mattip
Copy link
Member
mattip commented Aug 10, 2020

ppc64le CI build failure unrelated. I added a release note. Could one of the reviewers take a look?

@rossbar rossbar self-requested a review August 10, 2020 18:28
rossbar
rossbar approved these changes Aug 10, 2020
View reviewed changes
Copy link
Contributor
@rossbar rossbar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM - thanks @mattip for the release note

@mattip mattip merged commit 0311c93 into numpy:master Aug 13, 2020
@mattip
Copy link
Member
mattip commented Aug 13, 2020

Thanks @sidhant007

@charris charris mentioned this pull request Oct 10, 2020
Closed
20 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rossbar rossbar rossbar approved these changes

@eric-wieser eric-wieser Awaiting requested review from eric-wieser

@anirudh2290 anirudh2290 Awaiting requested review from anirudh2290

Assignees
No one assigned
Labels
01 - Enhancement component: numpy._core
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

recarray fromfile doesn't support file like objects (Trac #1911)
6 participants
@sidhant007 @mattip @eric-wieser @rossbar @anirudh2290 @charris
0