Contribute to PyTorch project

[federicopozzi33]

Hi Philip,

I would like to contribute to the PyTorch project. I use the PyTorch ecosystem very often (the Python API), and it could be very interesting and instructive for me to contribute and dive deeper into the codebase.

Since I have never contributed to any open source project, and I use mainly Python, I think it should be easier for me to get started with the torchvision project.

I have read the contributing document here, and I'm quite familiar with tools like black, flake8, pre-commit hooks considering that I use them every day at work.

Do you think I can start with issues like this, for instance?

Cheers,

Federico

[pmeier]

Hey Federico 👋

I would like to contribute to the PyTorch project. I use the PyTorch ecosystem very often (the Python API), and it could be very interesting and instructive for me to contribute and dive deeper into the codebase.

That is awesome to hear 💯 Contributing to OSS projects was my way in, so can recommend this wholeheartedly.

Since I have never contributed to any open source project, and I use mainly Python, I think it should be easier for me to get started with the torchvision project.

If you are looking to contribute to projects from Meta, you are right to not start with PyTorch core. That is kind of a beast that is notoriously hard to understand. Plus, the core is written in C++ that is only wrapped in Python, so not much you can do there (same for me BTW). The domain libraries like torchvision are indeed a better starting point. From looking at your profile, it seems you are doing some work / research on vision projects so that is a nice fit too.

Do you think I can start with issues like this, for instance?

Depends. Are you familiar with the fundamentals of git and the GitHub workflow? If yes, you can go ahead. Just comment on the issue and I can assign you so no one else takes this.

Otherwise, we may want to break this task into multiple steps to "ease you in".

[federicopozzi33]

Depends. Are you familiar with the fundamentals of git and the GitHub workflow? If yes, you can go ahead. Just comment on the issue and I can assign you so no one else takes this.

Otherwise, we may want to break this task into multiple steps to "ease you in".

I'm familiar with git. I have never used GitHub workflow (Actions?), but I know Drone CI and use it in my daily workflow.

Do you think that reading this could be a good starting point?

[pmeier]

I'm sorry, I should have been more clear. By "GitHub workflow" I mean: do you know how to

• fork torchvision,
• create a feature branch,
• make your changes,
• push these changes to your fork, and
• open a PR with these changes?

For someone new to the whole OSS thing that can be already quite overwhelming. If you are familiar with all of that, I don't see an issue starting with a "larger" issue. Otherwise we might want to make "baby steps" on the feature side to familiarize you with the process.

[federicopozzi33]

I'm sorry, I should have been more clear. By "GitHub workflow" I mean: do you know how to

• fork torchvision,
• create a feature branch,
• make your changes,
• push these changes to your fork, and
• open a PR with these changes?

For someone new to the whole OSS thing that can be already quite overwhelming. If you are familiar with all of that, I don't see an issue starting with a "larger" issue. Otherwise we might want to make "baby steps" on the feature side to familiarize you with the process.

Yes, I should be fine with all that stuff.

Before I start working on the PR, there are some branch name or commit message conventions I should know about / to follow?

I looked at some PRs a little bit, but I can't spot any defined schemas (e.g. conventional commits).

[pmeier]

Yes, I should be fine with all that stuff.

👍

Before I start working on the PR, there are some branch name or commit message conventions I should know about / to follow?

Nope, you can go wild 🙂 Make sure the PR title is expressive, but not even that is set in stone. We can always change later.

Still, you might want to pick another transform to port. In pytorch/vision#5555 someone else is doing RandomCrop, which also needs Pad. They might need to port that as well.

[pmeier]

I think pytorch/vision#5523 would be a good fit.

[pmeier]

Oh wow. We independently picked the same issue 😆

[pmeier]

Hey @federicopozzi33. The PR is great, thanks a lot 💯 Normally, we would merge soon and fix nitpicks later to avoid giving contributors a bad experience. Given that you asked for my help here, I can also add all the comments on the PR for you to fix instead of doing it myself. Do you want me to do that or should we go ahead and merge?

[federicopozzi33]

Hi Philip,

just add all the comments and I'll try to fix them!

Anyway, this evening I will add tests for p=0, as indicated in the PR.

[federicopozzi33]

Hi Philip,

Something strange it's happening on the PR I'm working on.

I have just removed some type checks from tests, but some CI checks are failing now.
In particular, lint is failing.

I always run pre-commit hooks and mypy before pushing. Actually, no errors come up if I run on my machine.
Then, the error seems to be related to something I didn't change.

Can you please take a look?

EDIT: I've just realized what's happening. torch/vision#5598 fixes the problem I described above.
In general, should I constantly rebase my branch on master or it's something you do just before the PR is merged?

[pmeier]

I always run pre-commit hooks and mypy before pushing.

That should be enough. What happened here is that mypy==0.940 was released and since we don't pin the version in CI (I'm trying to change that) it found an error that was not previously there.

In general, should I constantly rebase my branch on master or it's something you do just before the PR is merged?

No, you don't need to constantly rebase and yes we make sure that your branch is up to date before merge. In fact, the repository is setup in a way that only up to date branches are allowed to be merged.

If you hit errors in CI that seem to be unrelated to your PR, I would ignore them at first. If you are unsure, feel free to ask if your PR is causing this. As maintainer we have a better overview what CI jobs are currently failing and which should be passing.

[federicopozzi33]

I always run pre-commit hooks and mypy before pushing.

That should be enough. What happened here is that mypy==0.940 was released and since we don't pin the version in CI (I'm trying to change that) it found an error that was not previously there.

In general, should I constantly rebase my branch on master or it's something you do just before the PR is merged?

No, you don't need to constantly rebase and yes we make sure that your branch is up to date before merge. In fact, the repository is setup in a way that only up to date branches are allowed to be merged.

If you hit errors in CI that seem to be unrelated to your PR, I would ignore them at first. If you are unsure, feel free to ask if your PR is causing this. As maintainer we have a better overview what CI jobs are currently failing and which should be passing.

Ok, perfect.

Do you think I should wait for the PR to be closed before working on another issue?

[pmeier]

Do you think I should wait for the PR to be closed before working on another issue?

Unless the PRs depend on each other, feel free to have as much open stuff as you like. But be aware that the more stuff you have open, the harder it is juggle the mental models.

I'll try to get the first PR merged today. For the future: please resolve conversations that you addressed. I saw you added commits, but since all my comments were still open, I was under the impression that you are still working on it. Only when you requested Nicolas' review, I actually looked at the code and realized you are actually waiting for us.

[pmeier]

No, you don't need to constantly rebase and yes we make sure that your branch is up to date before merge.

I know I said this, but one exception to it is if GitHub detects conflicts. In that case, please update your branch. This is a lot easier for you than for us and most of the time there is no actual conflict but rather another commit changed something in the same place as you.

[pmeier]

Congratulations on your first successful PR on torchvision pytorch/vision#5563 🎉 I'm going to close this issue now, but feel free to open a new one if you need more guidance.

In general though, you can simply ping me on issues / PRs directly. I might not always be the person you need to talk to, but I can probably get you in touch 🙂