Skip to content

Conversation

@lc0
Copy link
Contributor

@lc0 lc0 commented Oct 8, 2021

Accordingly to pytorch/pytorch#59077 this PR migrates from np.frombuffer to pytorch.frombuffer and closes #4552

cc @pmeier

Copy link
Contributor

@datumbox datumbox left a comment

Choose a reason for hiding this comment

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

@lc0 Thanks a lot for the quick PR. Overall it looks great. I believe you removed the use of numpy frombuffer from all places.

@pmeier there is one more below, do you think it's worth changing it?

parsed = np.frombuffer(data, dtype=m[1], offset=(4 * (nd + 1)))

@datumbox datumbox requested a review from pmeier October 8, 2021 16:36
@lc0
Copy link
Contributor Author

lc0 commented Oct 8, 2021

vision/torchvision/datasets/mnist.py

I think we can also refactor that use-case. I was just not sure if we loose readability by doing so or no. So what do you think @pmeier

Copy link
Contributor

@prabhat00155 prabhat00155 left a comment

Choose a reason for hiding this comment

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

Thanks @lc0, looks good to me.

@prabhat00155 prabhat00155 merged commit a9940fe into pytorch:main Oct 8, 2021
@github-actions
Copy link

github-actions bot commented Oct 8, 2021

Hey @prabhat00155!

You merged this PR, but no labels were added.

@pmeier
Copy link
Contributor

pmeier commented Oct 9, 2021

there is one more below, do you think it's worth changing it?

Yes, I think so. I'll send a PR.

facebook-github-bot pushed a commit that referenced this pull request Oct 14, 2021
Summary: Co-authored-by: Prabhat Roy <[email protected]>

Reviewed By: fmassa

Differential Revision: D31649965

fbshipit-source-id: bac1178be3902694af3b2b3d195e0abfe0d640e5
mszhanyi pushed a commit to mszhanyi/vision that referenced this pull request Oct 19, 2021
cyyever pushed a commit to cyyever/vision that referenced this pull request Nov 16, 2021
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.

Use torch.frombuffer instead of np.frombuffer

4 participants