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

Accept numpy array view. #4147

Merged
merged 6 commits into from
Feb 18, 2019
Merged

Accept numpy array view. #4147

merged 6 commits into from
Feb 18, 2019

Conversation

trivialfis
Copy link
Member

close #3841 .

@trivialfis trivialfis changed the title [WIP] Accept numpy array view. Accept numpy array view. Feb 16, 2019
@trivialfis trivialfis requested a review from hcho3 February 16, 2019 12:02
_check_call(_LIB.XGDMatrixSetGroup(self.handle,
c_array(ctypes.c_uint, group),
c_bst_ulong(len(group))))
self.set_uint_info('group', group)
Copy link
Collaborator

@hcho3 hcho3 Feb 18, 2019

Choose a reason for hiding this comment

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

Are you sure this is equivalent to XGDMatrixSetGroup()? If so, can we add a test for groups?

Copy link
Member Author

Choose a reason for hiding this comment

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

Good point. SetInfo does a copy first, while SetGroup builds the group_ptr on the fly, which should be slightly faster. But other than this, the code is duplicated. It's a little bit tricky to add a test, since we can't get the group back without another function transforming group_ptr into the original group. We can postpone it to #4150. But I am happy to add such test if you think it's better.

@trivialfis trivialfis merged commit a985a99 into dmlc:master Feb 18, 2019
@trivialfis trivialfis deleted the fix/numpy-view branch February 18, 2019 14:21
hcho3 added a commit to hcho3/xgboost that referenced this pull request Feb 19, 2019
hcho3 added a commit that referenced this pull request Feb 20, 2019
* Revert "Accept numpy array view. (#4147)"

This reverts commit a985a99.

* Fix #4163: always copy sliced data

* Remove print() from the test; check shape equality

* Check if 'base' attribute exists

* Fix lint

* Address reviewer comment

* Fix lint
@lock lock bot locked as resolved and limited conversation to collaborators May 19, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[BLOCKING] [python] DMatrix setting incorrect values from sliced array
2 participants