SciRuby / daru

Data Analysis in RUby
BSD 2-Clause "Simplified" License
1.03k stars 139 forks source link

In Daru 0.1.6 Vector.new([5, 4, 3, 2, nil]).min / max breaks due to nil #386

Closed parthm closed 6 years ago

parthm commented 6 years ago

Tried upgrading to Daru 0.1.6 and got the following error:

/home/parth/.rbenv/versions/2.4.1/lib/ruby/gems/2.4.0/gems/daru-0.1.6/lib/daru/maths/statistics/vector.rb:156:in `sort': comparison of NilClass with -25500.0 failed (ArgumentError)
        from /home/parth/.rbenv/versions/2.4.1/lib/ruby/gems/2.4.0/gems/daru-0.1.6/lib/daru/maths/statistics/vector.rb:156:in `min'

Still trying to narrow this down to a smaller workable example. The app works fine with 0.1.5. As it analyzes lots of data I am unsure at this point as to whats causing the issue.

parthm commented 6 years ago

Got this narrowed down to change in behavior of min: This works with 0.1.5.

irb(main):001:0> require 'daru'

Install the reportbuilder gem version ~>1.4 for using reportbuilder functions.
=> limited output
 true
irb(main):002:0> Daru::VERSION
=> limited output
 "0.1.5"
irb(main):003:0> Daru::Vector.new([5, 4, 3, 2, nil]).min
=> limited output
 2
irb(main):004:0>

With 0.1.6 this fails:

irb(main):001:0> require 'daru'
=> limited output
 true
irb(main):002:0> Daru::VERSION
=> limited output
 "0.1.6"
irb(main):003:0> Daru::Vector.new([5, 4, 3, 2, nil]).min
ArgumentError: comparison of Integer with nil failed
        from /home/parth/.rbenv/versions/2.4.1/lib/ruby/gems/2.4.0/gems/daru-0.1.6/lib/daru/maths/statistics/vector.rb:156:in `sort'
        from /home/parth/.rbenv/versions/2.4.1/lib/ruby/gems/2.4.0/gems/daru-0.1.6/lib/daru/maths/statistics/vector.rb:156:in `min'
        from (irb):3
        from /home/parth/.rbenv/versions/2.4.1/bin/irb:11:in `<main>'
irb(main):004:0>
athityakumar commented 6 years ago

Acknowledged, @parthm. Thanks for opening this issue. I think this has crept in from a2702d7 - the Daru::Vector#min currently calls the default Array#sort without checking for nil. Changing the data = @data.data.to_a to data = @data.data.to_a.compact in Daru::Vector#min (and #max) should do it, I think.

parthm commented 6 years ago

Thanks for checking the commit @athityakumar . Just tested out max. Looks like this issue impacts max as well.

Daru 0.1.5

irb(main):002:0> Daru::VERSION
=> limited output
 "0.1.5"
irb(main):003:0> Daru::Vector.new([5, 4, 3, 2, nil]).max
=> limited output
 5
irb(main):004:0>

Daru 0.1.6

irb(main):002:0> Daru::VERSION
=> limited output
 "0.1.6"
irb(main):003:0> Daru::Vector.new([5, 4, 3, 2, nil]).max
ArgumentError: comparison of Integer with nil failed
        from /home/parth/.rbenv/versions/2.4.1/lib/ruby/gems/2.4.0/gems/daru-0.1.6/lib/daru/maths/statistics/vector.rb:97:in `sort'
        from /home/parth/.rbenv/versions/2.4.1/lib/ruby/gems/2.4.0/gems/daru-0.1.6/lib/daru/maths/statistics/vector.rb:97:in `max'
        from (irb):3
        from /home/parth/.rbenv/versions/2.4.1/bin/irb:11:in `<main>'
irb(main):004:0>
parthm commented 6 years ago

Closed due to bad click. Re-opening. Sorry.

v0dro commented 6 years ago

@athityakumar can you send a patch ASAP?

athityakumar commented 6 years ago

@v0dro - Sent a patch via PR #387. Please review. 😄

Ronaq13 commented 6 years ago

If it is not fixed in previous patch, then I would like to take this bug. I am a newbie, can you please guide.

parthm commented 6 years ago

@Ronaq13 , thanks for the offer to help. It's much appreciated. Since, @athityakumar is already made significant progress on this, may I suggest something else like issue #397 or #156 ? I am sure folks will be happy to help as needed. You could discuss your proposal on these (or any other) specific issues and submit a PR if there is consensus on the approach.