✔️ Linting Passed

All linting checks passed. Your pull request is in excellent shape! ☀️

_{Generated for commit: 86a4408. Link to the linter CI: here}

I was personally hoping for a quick and dirty list of commands that work in most scenarios rather than a detailed explanation of how to deal with lock-file conflicts in general. I now realize that it may be more complicated than I naively thought ...

Maybe we should put something like this in the developer tips explaining some of the caveats?

# merge upstream/main
git pull upstream main --no-rebase
# there are conflicts so say keep the version from upstream/main
git checkout --theirs build_tools
git add build_tools
git merge --continue
# rerun lock-file update if needed
python build_tools/update_environments_and_lock_files.py

I would say that for most contributors using @scikit-learn-bot please update lock-files is the easiest thing to do.

Maybe the list of commands can be made more robust (for example you probably want to keep changes to build_tools/update_environment_and_lock_files.py and solve conflicts there if any) and the bot lock-file enhanced to deal with the simple kind of conflicts ...

I was personally hoping for a quick and dirty list of commands that work in most scenarios rather than a detailed explanation of how to deal with lock-file conflicts in general.

I get your point @lesteve. If you're doing this more often, you want everything in one cell for a quick copy paste. At the same time I think less experienced people can benefit from the detailed explanations to understand the whys.

Do you think we can put everything in here: the quick-and-dirty first followed by the step-by-step explanation? I feel that the latter is quite general and I am not aware of any other caveats that I could put into the developer tips (which are quite hidden). But I can also move it over there and leave a link here?

Honestly this is not easy to get the balance right and opinions probably vary. My personal opinion is that the contributing doc is already quite long. Also in this particular case only a small fraction of contributors will have to deal with lock-files and only a fraction of them will have conflict in their lock-files.

What I had in mind was a snippet that works in most cases that scikit-learn maintainers could point to or find when needed. Comments can certainly be added in the snippet to make things more clear.

Not entirely sure this is a great idea, but you could imagine (if the list of commands I mentioned can be made a bit more robust) that one day there is something like this @scikit-learn-bot please update lock-files --fix-conflicts ...

For completeness, the kind of caveat I can imagine with my snippet:

• git checkout --theirs build_tools will take the versions from main for all the build_tools/. I think it would be better to apply this only to environment and lock files
• this may not be enough to fix all the conflicts and you need user intervention for this. It would only work if the only conflicts are in the environment and lock files.
• python build_tools/update_environment_and_lock_files.py does not work on Windows, this is why @Charlie-XIAO got motivated to work on the lock-file bot idea.

Just to add a point, I think we want to recommend contributors to use the comment to trigger the bot instead of updating on their own, because we are confident that the bot commit does nothing malicious but human commits may do (and we cannot tell because lock files are not reviewable).

I can see your point and agree to have it short only then, @lesteve. I will make up my mind on how to do that after PyData.

Just to add a point, I think we want to recommend contributors to use the comment to trigger the bot instead of updating on their own, because we are confident that the bot commit does nothing malicious but human commits may do (and we cannot tell because lock files are not reviewable).

A bit off-topic but I am not overly worried about security implications (I could well be wrong ...). Personally, I do browse the lock-file diff to double-check nothing weird is happening. I would not say that lock-files are unreviewable but I would agree it is easy to miss something ...

But now I am doubting why we want to keep exactly those files from main, that are auto-generated.

We want to resolve the conflicts in the simplest possible way, knowing that we will regenerate the lock-files afterwards. In other words it does not really matter which version we pick (from main of from our branch), since we will update the lock-files again by running the script build_tools/update_environments_and_lock_files.py.

Rereading the commands I think we should be consistent in the git checkout and git add i.e. something like:

environment_and_lock_files=$(ls build_tools/*/*.lock build_tools/*/*environment.yml build_tools/*/*lock.txt build_tools/*/*requirements.txt)
git checkout --theirs $environment_and_lock_files
git add $environment_and_lock_files

Thanks for clarifying, @lesteve. After your comment and after talking with @adrinjalali, I have understood that here we're only dealing with resolving the lock file conflicts, not considering the reasons that lead to it (possible changes of the project wide dependency files), and I could make the separation in my mind.

I have updated the docs, please have a look.

However, I didn't manage to make it work with the $environment_and_lock_files when I tested it. One reason it that git checkout --theirs needs to get the paths separated by a space, not by a new line. But also after adjusting this, it couldn't take apart the environmental variable that way it is supposed to. This is why I passed the paths to git checkout --theirs and to git add individually.

Thanks, I set auto-merge. This looks good, and if the need arises this can always be improved in the future.

We have an issue with our docbuild on circleci which is unrelated to this PR, and reproduced on other PRs as well.

With #30333 merged, the CircleCI issue should be fixed now, I merged main so hopefully this should get merged when CI is green.