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 default order for Daru::DataFrame (issue #130) #329

Merged
merged 4 commits into from
Apr 12, 2017

Conversation

athityakumar
Copy link
Member

Rspec test added, has been written in the older format to main consistency. This will be easier to change while re-writing all tests to modern specifications file-wise, rather than modernizing specs for individual functions. RDoc documentation has also been added.

Tackles issue #130

Rspec test added, has been written in the older format to main consistency. This will be easier to change while re-writing all tests to modern specifications file-wise. RDoc documentation has also been added.
@@ -2278,8 +2289,10 @@ def initialize_from_array source, vectors, index, opts

case source.first
when Array
vectors = (0..source.size-1).to_a if vectors.nil?
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why not ||=?

@athityakumar
Copy link
Member Author

athityakumar commented Apr 7, 2017

@zverok @v0dro - After closer inspection of the logs of the last successful build and current failing build of master branch in Travis CI, I observe that bundle install used to install rubocop v0.47.1 (build passing) and is now installing rubocop v0.48.0 (build failing). Have a look at the respective releases of rubocop gem.

We can specify rubocop (~>= 0.47.1) in the Gemfile.lock and remove the Gemfile.lock from .gitignore. Do you think this would fix the build error, or could it be something else?

@zverok
Copy link
Collaborator

zverok commented Apr 7, 2017

We can specify rubocop (~>= 0.47.1) in the Gemfile.lock and remove the Gemfile.lock from .gitignore. Do you think this would fix the build error, or could it be something else?

Instead of that, we can fix current rubocop version offences on each PR. They are reasonable, and being a small project we can't expect dedicated volunteers to "update rubocop" in separate PR. So whoever met new offences first is that unfortunate person who fixes them :)

@zverok
Copy link
Collaborator

zverok commented Apr 7, 2017

(but you can exclude Rakefile and Gemfile from rubocop-checked files, of course)

@athityakumar
Copy link
Member Author

@zverok - Alright, I'm taking one for the team. :)

Rubocop offenses are fixed, and build is finally successful. It does feel good to see the Build passing and green tick again. 🎉

Copy link
Collaborator

@zverok zverok left a comment

Choose a reason for hiding this comment

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

Good work 👍

#
# # =>
# # #<Daru::DataFrame: bat_man (4x2)>
# # 0 1
Copy link
Member

Choose a reason for hiding this comment

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

Given your example, shouldn't [6,7,8,9] be the 1st vector?

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks for pointing this out - fixed! 😄

Copy link
Member

Choose a reason for hiding this comment

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

Didn't you use actual executed code for the example? Its better if you do that.

Copy link
Member Author

Choose a reason for hiding this comment

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

I hadn't used the actual executed code in the initial commit (I had copy-pasted the previous rdoc example - my bad). I've added the output I get in the irb console while initializing the DataFrame given in the rdoc example, in the subsequent commit.

@athityakumar athityakumar mentioned this pull request Apr 10, 2017
3 tasks
@v0dro v0dro merged commit 47d9da0 into SciRuby:master Apr 12, 2017
parthm added a commit to parthm/daru that referenced this pull request Aug 26, 2017
parthm added a commit to parthm/daru that referenced this pull request Aug 26, 2017
v0dro pushed a commit that referenced this pull request Aug 26, 2017
Shekharrajak pushed a commit to Shekharrajak/daru that referenced this pull request Sep 5, 2017
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