@jakirkham might be interested.

A first comment before a proper review: I think it would be nice to point towards this new section from the reading the existing code base and the performance tips for the cython developer sections. It could be a simple "for more information, see section ...".

@ArturoAmorQ: feel free to open a pull-request on my branch if you think this can be better worded and if it is easier for you. 👍

Relevant notes from @da-woods people might be interested reading: https://cython-guidelines.readthedocs.io/en/latest/

@ArturoAmorQ: feel free to open a pull-request on my branch if you think this can be better worded and if it is easier for you. +1

You can cherrypick my commit here: ArturoAmorQ/scikit-learn@32648c5 (#2)

@da-woods: how interested are you to merge such notes (with yours) (in Cython's documentation)?

@da-woods: how interested are you to merge such notes (with yours) (in Cython's documentation)?

I think we are interesting in merging more of this sort of this into the Cython documentation. It probably won't be all my notes - I know there's a few of my opinions (e.g. my abiding dislike of cpdef functions) that Stefan doesn't necessarily agree with so I'd rather keep those somewhere where it's "just my opinion".

I still think duplication doesn't hurt so there's no reason not have this section in PandasScikit-learn and then see what we can pick across.

I tend to agree with all of your notes.

While it is useful to have Cython's developers and users (e.g. you and me here) share their experience, tips and conventions about the language, I think everyone would benefit from having them unified at one canonical place (ideally Cython's docs).

Do you think it possible to identify and extract non-opiniated, factual elements from both yours and those notes and integrates them in Cython's docs ?

Also, were you referring to scikit-learn instead of pandas in your previous comments ?

Do you think it possible to identify and extract non-opiniated, factual elements from both yours and those notes and integrates them in Cython's docs ?

I'll try to make a start on that in the next few weeks (probably bit-by-bit)

Also, were you referring to scikit-learn instead of pandas in your previous comments ?

Oops yes sorry! I got temporarily dislocated!

Added the "no changelog" label, as I think this wouldn't get added.

I like it and find it useful as is. As such, should we try and merge this as soon as this round of comments is addressed? It seems like work on this PR could go on for a long time because there is so much to know. However the longer it is unmerged, the longer people won't profit from what is already here. Can always do a second, third, fourth PR to keep working on this. WDYT?

Yes, I agree. I think Jérémie is busy with the 1.2.2 release, but other maintainers can give their +1.

I am waiting for this PR to get 2 approvals with potential proposals for subsequent PRs improving the section.

Can always do a second, third, fourth PR to keep working on this

I'm also in favor of merging a good enough version and improve in subsequent PRs

@jeremiedbb: I think I've treated all of your comments.

Reading the discussions in #25780, I think it will be worth adding a bullet point about including cnp.import_array(), but it can be done in another PR

I like it and find it useful as is. As such, should we try and merge this as soon as this round of comments is addressed? It seems like work on this PR could go on for a long time because there is so much to know. However the longer it is unmerged, the longer people won't profit from what is already here. Can always do a second, third, fourth PR to keep working on this. WDYT?

I agree 💯 It is better to have 4 smaller PRs and to iterate faster.

Thanks

Thanks everyone! I hope this will help people getting started using Cython for scikit-learn.