SciRuby / daru

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

add method set to category.rb #446

Closed rohitner closed 4 years ago

rohitner commented 6 years ago

created a PR for discussion on issue #390

v0dro commented 6 years ago

@lokeshh ping!

rohitner commented 6 years ago

@lokeshh does it LGTY ?

rohitner commented 6 years ago

Can you please check :sweat_smile: , it is getting rectified when modify_category_at is called. But I should correct it. :+1:

irb(main):026:0> v = Daru::Vector.new([1,2,3])
=> #<Daru::Vector(3)>
   0   1
   1   2
   2   3
irb(main):027:0> vc = v.to_category
=> #<Daru::Vector(3):category>
   0   1
   1   2
   2   3
irb(main):028:0> vc.send(:set, 5, 6)
=> #<Daru::Vector(4):category>
   0   1
   1   2
   2   3
   5   6
irb(main):029:0> vc.to_ints
=> [0, 1, 2, 3]
irb(main):030:0> vc[1]=6
=> 6
irb(main):031:0> vc.to_ints
=> [0, 3, 2, 3]
rohitner commented 6 years ago

ping! @lokeshh

rohitner commented 6 years ago

I doubt whether set works for multiple indices. Please help.

rb(main):003:0> v = Daru::Vector.new([1,2,3])
=> #<Daru::Vector(3)>
   0   1
   1   2
   2   3
irb(main):004:0> v.send(:set,5,6,2)
ArgumentError: wrong number of arguments (given 3, expected 2)
    from /home/rohitner/.rbenv/versions/2.4.2/lib/ruby/gems/2.4.0/gems/daru-0.2.0/lib/daru/vector.rb:1621:in `set'
    from (irb):4
    from /home/rohitner/.rbenv/versions/2.4.2/bin/irb:11:in `<main>'
irb(main):005:0> 

@lokeshh can you reproduce this?

rohitner commented 6 years ago

ping:exclamation: @lokeshh

lokeshh commented 6 years ago

You need to pass an array like v.set [5, 6], 2. And sorry for replying late.

rohitner commented 6 years ago

Can you please check if I have made the required changes?

v0dro commented 6 years ago

@lokeshh is this good for merge?

lokeshh commented 6 years ago

@v0dro No not yet but almost done.

rohitner commented 6 years ago

@lokeshh are there any other changes/additions needed?

v0dro commented 6 years ago

@rohitner such a huge discussion for a simple PR is not a good thing at all. You should focus more, understand all the issues at once and submit a PR that is in spirit of the rest of the gem. Most of @lokeshh 's comments so far are mostly about things that are already there in daru, but which you were unaware of. If you explore the codebase more you can find them on your own, and you should.

Please note that all of us are volunteers and we cannot continuously point out nitty gritties.