You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The reason will be displayed to describe this comment to others. Learn more.
I think, it will not work properly for dataframe having multi indexes. Can you please try with multi index and find the issue? We can have this method for dataframe having only index (not multi index).
I tried it with multi indexes and found two different bugs which needs to be managed separately. This method will work with multi indexes only when a single multi-index is provided
I think there is a bug in #pos method of Daru::MultiIndex when multiple multi-indexes are provided. access_row_tuples_by_index will work in this case when the issue with pos method will be resolved.
#pos method is working fine in the below case but #access_row_tuples_by_indexs gives error as it uses index for creating new rows but the provided index is not actually the valid index. It is used to get the positions.
The reason will be displayed to describe this comment to others. Learn more.
I think, the multi index issue must be handled in separate PR. This PR looks good. It would be better if you had added example along with above the method documentation.
Note: in dataframe.rb, it is possible to use instead:
def access_row_tuples_by_indexs *indexes
get_sub_dataframe(indexes, by_position: false).map_rows(&:to_a)
end
It is about 10 times slower that the current reverted code but, among the two bugs found by @Prakriti-nith , it is not affected by the second one (ie I have:
Thanks @paisible-wanderer for informing us. @Prakriti-nith , please use similar thing for dataframe having multi index and test it for different cases.
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
#462
This PR aims to revert back the changes for
access_row_tuples_by_indexs
method.