[python-package] do not copy column-major numpy arrays when creating Dataset from list of arrays

[jmoralez]

Following the suggestion in #6751 (comment) this changes the signature of the C API LGBM_DatasetCreateFromMats function to take an array of integers as is_row_major, indicating the layout of each matrix. Also updates the lightgbm.Dataset.__init_from_list_np2d method to provide this setting and avoid copying the arrays in the list.

[jmoralez]

This is a breaking change in the C API, so I can maybe split this into two PRs, one making the change to the C API only and mark it as a breaking change, and then implement the logic in the python package and mark it as efficiency. WDYT @StrikerRUS @jameslamb?

[jameslamb]

Thanks for this. I think we should NOT break the ABI just for this purpose. What do you think about introducing a new function in the C API like LGBM_DatasetCreateFromMatsMixedLayouts()?

Most of the implementation of LGBM_DatasetCreateFromMats() could be moved into this function, and then LGBM_DatasetCreateFromMats() could internally construct a vector of is_row_major and call this new function.

You will probably have a better recommendation for the name, but what do you think about this general idea?

In my opinion it's worth growing the C API a little bit in exchange for not breaking other projects that link against lib_lightgbm. I found a few of those on GitHub (with this search):

• lightgbm-rs (code link)
• feedzai-openml-java (code link)
• lightgbm4j (code link)
• LightGBM.NET (code link)

And we can use the docs on the function to clearly describe what the difference is.

[jameslamb]

This is a breaking change in the C API, so I can maybe split this into two PRs, one making the change to the C API only and mark it as a breaking change, and then implement the logic in the python package and mark it as efficiency

Even if we do decide to keep this as a breaking change, I'd be ok with it being one PR. The changes are so tightly related to each other and the total size of code changes is fairly small. I think it's ok to have a single one under the breaking category. Also the others are under efficiency so people will understand from the release notes that lightgbm makes fewer copies for numpy input... think that's enough.

[StrikerRUS]

Thank you very much, LGTM!

I think that one breaking PR is OK. We already have some breaking changes for next release , so this tiny breakage shouldn't be the only problem for other packages maintainers. They will already have to check Breaking section of changelog carefully, and changing int to int[] shouldn't be a big headache.

[jameslamb]

They will already have to check Breaking section of changelog carefully, and changing int to int[] shouldn't be a big headache.

Ok, let's leave it as an ABI-breaking change. We are not making ABI stability guarantees here yet anyway. I do think we should consider that at some point... maybe as part of #6774, we could start using the SOVERSION property in CMake to communicate ABI changes (CMake docs).