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

Adds option to display all data on inspect #476

Conversation

paisible-wanderer
Copy link
Contributor

Hello,

By default, using inspect on dataframe/vector/other will not display the whole data.

I have added an option that I think can be useful.

vect = Daru::Vector.new((1..16))

# before
vect
# => #<Daru::Vector(16)>
#    0   1
#    1   2
#    2   3
#    3   4
#    4   5
#    5   6
#    6   7
#    7   8
#    8   9
#    9  10
#   10  11
#   11  12
#   12  13
#   13  14
#   14  15
#  ... ...


# after
puts vect.inspect(all: true);nil
# #<Daru::Vector(16)>
#    0   1
#    1   2
#    2   3
#    3   4
#    4   5
#    5   6
#    6   7
#    7   8
#    8   9
#    9  10
#   10  11
#   11  12
#   12  13
#   13  14
#   14  15
#   15  16
# => nil

Let me know if there is an better way, or any improvement I can make to this PR.

Copy link
Member

@Shekharrajak Shekharrajak left a comment

Choose a reason for hiding this comment

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

Thanks for pointing out this. I think we can extend it for displaying the number of columns as well(for dataframe).
So we can have Daru::Formatters::DataFrame module and Daru::Formatters::Vector module where these value will be present globally and can be set using method like :

Daru::DataFrame.inspect_max_rows = 10
Daru::DataFrame.inspect_max_colms = nil
Daru::Vector.inspect_max_rows = nil 

def inspect spacing=10, threshold=15
#
# @option [Boolean] all: when true, will remove thresholding (and will override any value set for threshold)
def inspect spacing=10, threshold=15, all: false
Copy link
Member

Choose a reason for hiding this comment

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

I think we should have a method that can set the threshold for Daru::DataFrame and Vector to display maximum number of rows and columns.

So I believe we should have a method like

Daru::DataFrame.inspect_max_rows = nil # to load all the rows

Default value will be set to 15.

What do you think ? @paisible-wanderer

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hum... It think it could be a good idea.

On the other hand, I don't really see myself using the global options: the default values in daru are already ok; it is just that sometimes I want to inspect/debug and need to check some of the vectors/dfs fully.

Copy link
Member

Choose a reason for hiding this comment

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

In that case, you can mention the number of rows in the threshold option, itself.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You are totally right! Somehow, it did not occur to me (I was just setting threshold to 10_000_000_000).

I still think that the :all option makes (at least semantically) some sens, but I don't think it is that useful.

It think it would be best to simply refuse & close this PR, sorry for the trouble.

Copy link
Member

Choose a reason for hiding this comment

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

Thanks for update @paisible-wanderer ! It was a nice discussion. I learn, how we can improve formatter module in daru. Surely in future, this will be helpful.

@Shekharrajak
Copy link
Member

Closing after this comment : #476 (comment)

v0dro pushed a commit that referenced this pull request May 30, 2020
* Add support for options to Daru module

#476

#502

* Use the max_rows option on the terminal

* fix typo in table formatter
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