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

Insert column data at specified location #525

Conversation

cyrillefr
Copy link
Contributor

  • new method insert_vector in lib/daru/dataframe.rb (based on add_vector)
  • added specs

Aims to resolve issue #507

 - new method insert_vector
 - added specs
expect(subject).to eq df
end

it "raises error for data array being too big" do
Copy link
Member

Choose a reason for hiding this comment

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

Isn't it Argument Error ?

}.to raise_error(IndexError)
end

it "raises error for invalid index value" do
Copy link
Member

Choose a reason for hiding this comment

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

Isn't it IndexError ?

}.to raise_error(ArgumentError)
end

it "raises error for invalid source type" do
Copy link
Member

Choose a reason for hiding this comment

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

It would be better to provide some message/hint along with error, so that user can fix it error .

@@ -545,6 +545,17 @@ def add_vector n, vector
self[n] = vector
end

def insert_vector n, name, source
Copy link
Member

Choose a reason for hiding this comment

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

  • Why it is always necessary to provide name of the index ?

  • Please make the argument names clearly like: use nthColumn instead of n

@@ -545,6 +545,17 @@ def add_vector n, vector
self[n] = vector
end

def insert_vector n, name, source
raise ArgumentError unless source.is_a? Array
vector = Daru::Vector.new(source, index: @index, name: @name)
Copy link
Member

Choose a reason for hiding this comment

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

Can we have choice to pass vector directly to be inserted (think about other ways as well)?

raise ArgumentError unless source.is_a? Array
vector = Daru::Vector.new(source, index: @index, name: @name)
@data << vector
@vectors = @vectors.add name
Copy link
Member

Choose a reason for hiding this comment

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

Can't we do @vectors = @vectors.insert(n, name) simply?

@v0dro v0dro merged commit 1d1dec1 into SciRuby:master May 30, 2020
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