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.

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

fix typeinfo for sparse-matrix/vector display #30589

Merged
merged 8 commits into from
Jan 15, 2019
Merged

fix typeinfo for sparse-matrix/vector display #30589

merged 8 commits into from
Jan 15, 2019

Conversation

stevengj
Copy link
Member

@stevengj stevengj commented Jan 4, 2019 •
edited
Loading

When implementing #30575, I noticed that sparse matrices and vectors were not setting the typeinfo property of the IOContext, despite the fact that they print the element type. This PR fixes that.

@stevengj stevengj added sparse Sparse arrays display and printing Aesthetics and correctness of printed representations of objects. labels Jan 4, 2019
@stevengj stevengj mentioned this pull request Jan 4, 2019
Closed
@stevengj
Copy link
Member Author

stevengj commented Jan 4, 2019

Looks like an unrelated CI failure for Travis OSX: The job exceeded the maximum log length, and has been terminated. (This error occurred while compiling, before any Julia code runs.)

@ViralBShah
Copy link
Member

Travis mac is facing the issue of brew trying to compile GCC. #30599 should fix it.

rfourquet
rfourquet approved these changes Jan 7, 2019
View reviewed changes
Copy link
Member

@rfourquet rfourquet left a comment

Choose a reason for hiding this comment

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

May be good to add a test, an easy one would be with the Bool after #30575 is merged.

@stevengj
Copy link
Member Author

Added tests using bool arrays.

@stevengj
Copy link
Member Author

Fixed bug where SparseMatrix printing was discarding the IOContext, and removed an unnecessary IOBuffer.

@stevengj
Copy link
Member Author

Pushed more test fixes.

@stevengj
Copy link
Member Author

CI is green, finally. Okay to merge?

@fredrikekre fredrikekre merged commit 7004841 into JuliaLang:master Jan 15, 2019
@stevengj stevengj deleted the sparse_typeinfo branch January 15, 2019 19:32
@ViralBShah
Copy link
Member

Should this also be backported to 1.0?

@stevengj
Copy link
Member Author

Probably not so essential to backport; most people wouldn't even notice it before the modified Bool printing, since sparse matrices can only store numbers.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@fredrikekre fredrikekre fredrikekre left review comments

@rfourquet rfourquet rfourquet approved these changes

Assignees
No one assigned
Labels
display and printing Aesthetics and correctness of printed representations of objects. sparse Sparse arrays
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@stevengj @ViralBShah @rfourquet @fredrikekre