SciRuby / daru

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

Adds support to missing data for where clause #302

Closed athityakumar closed 7 years ago

athityakumar commented 7 years ago

Fixes issue #246. Sample output for the building pass is shown below.

2.3.1 :002 > v= Daru::Vector.new([1,2,3,Float::NAN, nil])
 => #<Daru::Vector(5)>
   0   1
   1   2
   2   3
   3 NaN
   4 nil 
2.3.1 :003 > v.where(v.lt(4))

 => #<Daru::Vector(3)>
   0   1
   1   2
   2   3 
2.3.1 :004 > v.where(v.lt(2))

 => #<Daru::Vector(1)>
   0   1 
2.3.1 :005 > v.where(v.lt(3))

 => #<Daru::Vector(2)>
   0   1
   1   2 
2.3.1 :006 > v.where(v.gt(1))

 => #<Daru::Vector(2)>
   1   2
   2   3 

Ping @v0dro @zverok @lokeshh - Please review this PR when you're free. :smile:

athityakumar commented 7 years ago

Tests, as in using Travis CI or test-unit gem?

zverok commented 7 years ago

@athityakumar I've meant specs -- there are ton of them & you can run them locally by just rspec or rspec path/to/your/spec.rb

v0dro commented 7 years ago

@athityakumar I suggest you first go through the CONTRIBUTING file and understand our processes properly before submitting PRs. We're all volunteers and it saves our time if you abide by some formal processes.

v0dro commented 7 years ago

Also, submitting a PR without tests is not acceptable. Especially if you want to apply for GSOC.

athityakumar commented 7 years ago

@v0dro : Thanks for mentioning about the CONTRIBUTING.md file - I just read through it. I'll add the tests in a day, after including the other missing data types and let you know for reviewing the PR again.

athityakumar commented 7 years ago

@v0dro @zverok : The PR is ready for another review.

Updates -

(1) Thanks for the rspec and rubocop references. They really made testing the changes simpler. (2) Support added for other missing data types like Float:NAN and Float:NAN in string. (3) Tests have been written for Daru::Vector#where and Daru::DataFrame#where.

Any changes required?

athityakumar commented 7 years ago

@zverok : I've added the specific test that would have failed on the current master branch version of daru. As suggested, I've also added my test variables in letformat - however, it does seem a bit different / inconsistent with the rest of the tests in the spec/core/query_spec.rb file, as they don't use let format.

zverok commented 7 years ago

however, it does seem a bit different / inconsistent with the rest of the tests in the spec/core/query_spec.rb file, as they don't use let format.

Yes, part of the specs are just old and written without much attention. We want to have them rewritten in modern style with time, yet it was not done yet.

athityakumar commented 7 years ago

@v0dro @zverok : Made the required changes in tests. :smile: