-
Notifications
You must be signed in to change notification settings - Fork 63
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
When printing matrices, replace zeros by dots #1459
base: master
Are you sure you want to change the base?
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #1459 +/- ##
==========================================
- Coverage 88.10% 86.67% -1.43%
==========================================
Files 120 120
Lines 30045 30021 -24
==========================================
- Hits 26470 26021 -449
- Misses 3575 4000 +425 ☔ View full report in Codecov by Sentry. |
Hm, I have a different opinion. I don't like the dots. I don't like the superfluous
|
For me the application are biggish matrices over finite fields where I can see the structure (e.g. block diagonal etc.) at a glance with the "." printing but not with integers, e.g. here an example over
where I don't immediately see what's going on, but
makes it stand out. |
I find
rather obscure. To be honest, I am not too excited about this change. Maybe we could hide it behind a preference? |
Having a preferences for this sounds useful, just like for the unicode printing -- and just like for that, we should then have a way to temporarily turn it off, and also turn it off (or on, for testing the feature itself) in test suites |
This makes it easier to read many matrices, especially those which are relatively sparse. Before: julia> matrix(QQ, [42 0 0; 0 42 0; 0 0 42]) [42//1 0//1 0//1] [ 0//1 42//1 0//1] [ 0//1 0//1 42//1] After: julia> matrix(QQ, [42 0 0; 0 42 0; 0 0 42]) [42//1 . .] [ . 42//1 .] [ . . 42//1]
a38b660
to
cc58444
Compare
This makes it easier to read many matrices, especially those which are relatively sparse.
Before:
After:
This is breaking in so far as it will require changes to a bunch of doctests in Oscar.jl and probably other packages.
I expect this will be somewhat controversial, but I wanted to at least put it out there. To me it is subjectively a major improvement but I realize others may have a quite different opinion. But I figured we could at least discuss it.