I've been testing this on my fork to make sure this works: https://github.com/thomasjpfan/scikit-learn/pulls

The bot goes through every 10 minutes and checks to see if the labels needs updating.

Currently, it does not remove labels. It does make sense to remove labels to keep the up to date with the PR.

The interesting case is when updates to the PR causes the number of modules change to go above max-labels. In this case, should we remove all the labels or leave the current labels alone? I am +0.5 on removing all the labels in this case.

Personally, I would use them to make reviewing the PRs more manageable. For example, after reviewing a PR on trees, it would be more efficient for me to look at another PR on trees, since it is already on my mind.

Maybe @caioaao or @adrinjalali can speak more about the purpose of the labels.

Side note: maybe a "this touchs alot of modules" labels, i.e. "modules:many"?

Personally, I would use them to make reviewing the PRs more manageable. For example, after reviewing a PR on trees, it would be more efficient for me to look at another PR on trees, since it is already on my mind.

I agree, also useful to find stalled PR the same topic etc.

So assume we change module labels in some PR because the auto-labeling got it wrong, does it mean that this manual change would be overwritten by the auto-labeler in 10min? Ideally PRs with existing module labels should not be relabeled.

So assume we change module labels in some PR because the auto-labeling got it wrong, does it mean that this manual change would be overwritten by the auto-labeler in 10min?

I would want to fix the labeler in this case. Currently the label should only match based on the module.

Ideally PRs with existing module labels should not be relabeled.

This is the current behavior. The labeler will not remove any labels and will only add up to max-labels.

I think the concern is that a label can go stale. If a PR changes a module and then removes the changes, the old label will not apply.

Yeah. as @thomasjpfan says, it makes it easier for people to focus on one module for reviews (and to submit PRs, when labels are applied to issues).

It also makes it easier to have an overall idea of what's being done on which module, which ones are active, etc.

So what happens if we label a PR which touches an example or a user guide, to have that label? I guess this PR doesn't touch those labels, am I correct?

If a PR touches the user guide or examples, this PR doesn't do anything with them. Currently, this PR is fairly conservative. It never removes labels and only adds labels that it understands (up to maxLabels). By "understand", I mean the labels that are configured in .github/labeler.yml.

then let see how it works :)

I am a little nervous. lol

WATCH IT GO: https://github.com/scikit-learn/scikit-learn/runs/480438551?check_suite_focus=true

On the second run, it should go much quicker since they were labeled by this run.

woohoo, seems working, exciting!

Hmm so it errored in the middle of adding labels. Looking into it.

Hmm so it errored in the middle of adding labels. Looking into it.

Label overflow? )

More seriously, I don't fully understand the error but wouldn't this still be limited by Github API restriction? In which several tries might be needed to label it the first time given the amount of PRs.

Yea, it is hitting the github api limit. This would most likely fix itself after waiting.

yea it resolved itself: https://github.com/scikit-learn/scikit-learn/runs/480601102?check_suite_focus=true

I will make one more update to use per_page=100 when requesting for the list of PRs to limit the number of API calls.

This really helps reviewing PRs by topic. Thanks @thomasjpfan !