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

Fix numpy VideoGrain.length to be the byte length #68

Merged
merged 1 commit into from
Jan 31, 2020

Conversation

philipnbbc
Copy link
Contributor

@philipnbbc philipnbbc commented Jan 30, 2020

This issue resulted in a grain converted to v210 and written to GSF to have an incorrect size in the grdt block header.

The # type: ignore was required because mypy reported mediagrains/numpy/videograin.py:254: error: overloaded function has no attribute "fset". There is an open mypy issue related to this: python/mypy#6045.

Found as part of pivotal job https://www.pivotaltracker.com/story/show/169632693

@philipnbbc philipnbbc force-pushed the philipn-numpy-data-len branch from a289611 to 3ffa7f0 Compare January 30, 2020 15:17
@philipnbbc philipnbbc requested a review from a team January 30, 2020 15:25
@jamesba jamesba self-assigned this Jan 30, 2020
Copy link
Contributor

@jamesba jamesba left a comment

Choose a reason for hiding this comment

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

LGTM. One inline comment about maybe adding some explanatory comments to one line.

mediagrains/numpy/videograin.py Show resolved Hide resolved
@philipnbbc philipnbbc force-pushed the philipn-numpy-data-len branch from 3ffa7f0 to 31c39c9 Compare January 31, 2020 17:00
@philipnbbc philipnbbc force-pushed the philipn-numpy-data-len branch from 31c39c9 to 379c9bb Compare January 31, 2020 17:05
@philipnbbc philipnbbc merged commit 2d4cadc into master Jan 31, 2020
@philipnbbc philipnbbc deleted the philipn-numpy-data-len branch January 31, 2020 17:14
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 this pull request may close these issues.

2 participants