[python-package] Allow to pass Arrow array & Polars series as position

[maxzw]

Summary

All other metadata fields (label, weight, group, init_score) accept PyArrow ChunkedArray types as input, but position only accepts numpy 1-D array or pandas Series. This is inconsistent and forces users working with Arrow-native or Polars data to manually convert to NumPy before passing position data. This contributes to #6204.

Reference PRs (part of #6022)

• [python-package] Allow to pass Arrow array as groups #6166
• [python-package] Allow to pass Arrow array as weights #6164

Changes

• Followed existing logic from other metadata fields to add PyArrow ChunkedArray and Polars support to the position field
• Updated documentation
• Added unit tests

[jameslamb]

Thanks for working on this! Very supportive of this change.

I haven't reviewed yet, just a quick note... CI will be blocked until someone can review this PR to fix a couple issues: #7249. Sorry for the delay.

[jameslamb]

Thanks for this, at a glance it looks pretty good to me and I support this.

There are some test failures:

FAILED tests/python_package_test/test_basic.py::test_dataset_construction_overwrites_user_provided_metadata_fields - AssertionError: 
Arrays are not equal

(dtypes int32, float32 mismatch)
 ACTUAL: array([0, 1], dtype=int32)
 DESIRED: array([0., 1.], dtype=float32)
FAILED tests/python_package_test/test_engine.py::test_ranking_with_position_information_with_dataset_constructor - AssertionError: 
Arrays are not equal

(dtypes int32, int64 mismatch)
 ACTUAL: array([0, 1, 2, ..., 1, 3, 9], shape=(3005,), dtype=int32)
 DESIRED: array([0, 5, 9, ..., 5, 4, 8], shape=(3005,))
= 2 failed, 1486 passed, 15 skipped, 8 xfailed, 52 xpassed, 3395 warnings in 542.38s (0:09:02) =

(build link)

Can you please investigate those?

It'll be faster to run these tests locally yourself, which you can do like this:

# build lib_lightgbm (only needs to be re-run for C++ changes)
rm -rf ./build
cmake -B build -S .
cmake --build build --target _lightgbm -j4

# build and install lightgbm Python package
sh build-python.sh --precompile install

# run tests
pytest \
  tests/python_package_test/test_basic.py \
  tests/python_package_test/test_engine.py

Happy to review more thoroughly once the tests are passing. @ me if you need help.

[maxzw]

There are some test failures:

Can you please investigate those?

Oops 😄 They should be passing now!

[jameslamb]

@maxzw could you please update this to account for the changes from #7275 , and add polars inputs for position?

I'd be happy to review after that, I think we can get this into the next release.

[maxzw]

@maxzw could you please update this to account for the changes from #7275 , and add polars inputs for position?

I'd be happy to review after that, I think we can get this into the next release.

Should be up-to-date now!

[jameslamb]

Implementation looks great, thank you! Looks to me like it's closely following the patterns @borchero set up for Arrow data in previous PRs.

I just left some small suggestions about type hints and tests.

[jameslamb]

Suggested change

	_LGBM_PositionType = Union[
	List[int],
	# 'position' intentionally does not support 'list' inputs
	# ref: https://github.com/lightgbm-org/LightGBM/pull/5929#discussion_r1262646998
	_LGBM_PositionType = Union[

It's an intentional choice to not support list inputs for position: #5929 (comment)

Please revert this and anything else in this PR that allows for position to be a list.

And let's add a code comment to prevent anyone from trying to introduce this again in the future. I could imagine others seeing this as an inconsistency to be "fixed" and I don't want to rely on maintainers remembering that conversation thread.

[jameslamb]

In general, list inputs for array-like data are painful to support. I'd prefer that lightgbm not add any more of them.

[maxzw]

I've removed the support for list inputs 👍🏻

[jameslamb]

This test_dataset_construct_position() test looks great.

Could you please add one more test here ensuring that cases where position has duplicate or out-of-order values are handled correctly? I don't see a bug in your changes, but could imagine cases where some logic bug would produce the values 0, 1, 2, 3, 4 for a 5-element position even when that encoding is not correct.

A test case like position=pa.chunked_array([15, 15, 15, 27, 8, 9, 15]) or something would be a strong test of correctness. It'd also be a way to cover that difference between get_field("position") and .position that you called out in a comment in the set_position() body.

[maxzw]

Added a unit test that addresses these cases! 👍🏻

[jameslamb]

This is looking good to me, thanks!

Merging in master to get one more CI run with the changes from #7344 .

@borchero could you review too when you have time?

[jameslamb]

Love this description of how this works, thank you.