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

fixed pretty print format for matrices #500

Merged
merged 1 commit into from
Aug 5, 2016

Conversation

Aelphy
Copy link

@Aelphy Aelphy commented Apr 8, 2016

No description provided.

@Aelphy Aelphy force-pushed the feature/pretty_print_2d_fix branch from 16af7d1 to 5c88482 Compare April 8, 2016 19:31
@Aelphy Aelphy force-pushed the feature/pretty_print_2d_fix branch from 5c88482 to deb5648 Compare April 9, 2016 14:14
@translunar
Copy link
Member

Can you provide a little more information about what you fixed? Was there a bug report associated with this PR?

@Aelphy
Copy link
Author

Aelphy commented Apr 11, 2016

I am working on the problem of matrix read/write ruby interface. I have faced the problem, that matrix is printed via pry not like a normal matrix row under the row, but row right of the row.

I have found it very inconvenient. Moreover I have found there a TODO: to make pretty_print method really pretty. I have deleted one extra if - because self.dim > 2 is always false in that branch of conditions.

There were no any bug report associated.

@translunar
Copy link
Member

This is a tough one to evaluate. Usually we require a spec or something like that, but this issue seems more subjective. Can you provide some specific use-cases (examples) that have been improved?

@v0dro
Copy link
Member

v0dro commented Apr 12, 2016

You can also provide terminal output in the comments.

@Aelphy
Copy link
Author

Aelphy commented Apr 19, 2016

what was before:

pry -r './lib/nmatrix.rb'
[1] pry(main)> g = N[[2, 3, 4], [7, 8, 9], [1,2 , 3]]
=>
[
[2, 3, 4] [7, 8, 9] [1, 2, 3] ]

What I propose:

pry -r './lib/nmatrix.rb'
[1] pry(main)> g = N[[2, 3, 4], [7, 8, 9], [1,2 , 3]]
=>
[
[2, 3, 4]
[7, 8, 9]
[1, 2, 3]
]

@v0dro
Copy link
Member

v0dro commented Apr 23, 2016

looks good! how would this scale to dimensions greater than 2?

@Aelphy
Copy link
Author

Aelphy commented Apr 23, 2016

For the moment it fixes only the cases with 2d. I could think about greater dims.

@v0dro
Copy link
Member

v0dro commented Apr 23, 2016

You can try to imitate octave/matlab.

@translunar
Copy link
Member

Yeah, please check that it doesn't mess up the handling of greater dimensions. That's an area I struggled with when writing the pretty_print code originally.

@Aelphy
Copy link
Author

Aelphy commented Aug 5, 2016

I am very sorry for late reply. I have checked the code for dims grater than 3. And than analyzed conditions in branches of this method. I am sure, that my code only affects dim 2!

@Aelphy
Copy link
Author

Aelphy commented Aug 5, 2016

if self.shape.size > 1 and self.shape[1] > 100
...
elsif self.dim > 3 || self.dim == 1
...
else # it could be 0 , 2 and 3 here
if self.dim == 3
...
else # dim 2 and 0 also
...
end
end

@translunar translunar merged commit 7f3fa5d into SciRuby:master Aug 5, 2016
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.

3 participants