[python-package] Make Dataset.set_position() update self.position

[maxzw]

Make Dataset.set_position() update self.position consistently with the other set_* methods

See #7260 (comment)

Changes

• python-package/lightgbm/basic.py: one-line fix in Dataset.set_position — self.position = self.get_field("position") after set_field, mirroring set_weight.
• tests/python_package_test/test_basic.py: new test_set_position_updates_self_position_with_remapped_int32_values covering a non-identity remap on both the constructor path and set_position() on a constructed Dataset; existing position assertions in test_dataset_construction_overwrites_user_provided_metadata_fields updated to expect int32 remapped values.
• tests/python_package_test/test_engine.py: get_position check in the lambdarank test compared against get_field("position") plus dtype/shape assertion.

Backwards compatibility

Dataset.position after construct now reflects the C++-side int32 remapped values instead of the original user input. get_position() is unchanged in shape and semantics.

[jameslamb]

Thanks for the quick fix, I agree that we should do this. Code changes and tests look excellent!

One request... please, from this point forward, write the PR title, description, and any review discussion yourself. This PR's description looks to me like it was written by an LLM and so does the description in #7343.

For example I don't think this claim is accurate:

Flagged in #5929 (review) but never addressed.

I looked through that review and couldn't find discussion of this... pretty sure it was missed when we reviewed #5929. And you've written #5929 (review), the way GitHub formats a link preview when you link to a review comment, but the link is just to the PR in general.

If I'm wrong that that's LLM output, I sincerely apologize. Please understand, open source maintainers are being asked to review a lot of LLM output these days and even when it's all accurate, it tends to be more verbose than what a human would write... I personally am very tired of spending my limited open-source time reading and correcting such things.

[maxzw]

@jameslamb thanks for your quick response and the candid feedback!

One request... please, from this point forward, write the PR title, description, and any review discussion yourself. This PR's description looks to me like it was written by an LLM and so does the description in #7343.

It is partly written by an LLM, and I totally understand the fatigue. Please know that this is just a best-effort in the absence of a proper PR template or a dedicated PR-description guide. From this point forward I'll try to mimick the details/verbosity of previous PR's more closely. Please give me some time to adjust the current ones, before you spend any more time and energy in reading them 😃

[jameslamb]

Thanks so much.

Yeah we probably should have a PR template. I've found that people mostly ignore the issue templates we have so that has discouraged me from putting in effort there, but it's a fair point that we should offer some guidance for thoughtful contributors like you. Thanks for the suggestion 😊

verbosity

I want to say too... I'm happy to read long descriptions if they're full of information! I write long PR descriptions all the time, like #7340 and #7271 😅

I just mean that LLMs tend to use a lot of words when fewer could convey the same information. For example the Changes section in this PR's description could have just been:

## Changes

* updates `Dataset.position` to the constructed value, matching how e.g. `set_weight()` works
* adds tests on that behavior

I promise not to nitpick you on style, any description you write yourself will be just fine. I'm happy to invest time talking to you, but not to talk to an LLM through you.

Really appreciate you talking about this with me and making such great contributions!!