Submitting PR to fastai V2

(Aman Arora) #1

Submitting Pull Request to fastai V2

I am a Wiki thread! You can edit me. :slight_smile:

Creating a PR using Hub

  1. Install Hub from here
  2. Clone fastai dev repo to local
    hub clone https://github.com/fastai/fastai_dev
  3. Run tools/run-after-git-clone.
    tools/run-after-git-clone
  4. Checkout to new feature branch (this is where new feature/bug-fix should be made)
    git checkout -b <enter--relevant-feature-branch-name>
  5. Make changes to Jupyter Notebook (ie., add tests, update feature, add new feature, add documentation etc.)
  6. Save updated notebook
  7. Run notebook2script to convert notebook to python files
    python notebook2script.py --fname <enter-ipynb-notebook-name-here>
  8. Commit change
    git commit -m "done with feature"
  9. Fork the repo
    hub fork --remote-name=origin
  10. Push changes to forked repo
    git push origin <enter-branch-name-same-as-before>
  11. Create Pull Request
    hub pull-request
  12. Add PR description and you’re done!

Note: Steps 1-3 need to be only performed once when you fork fastai_dev for the first time. In subsequent PRs start from the step 4.

Updating a PR

If you want to change your code after a PR has been created you can do it by sending more commits to the same remote branch. For example:
git commit -m "updated the feature"
git push origin <enter-branch-name-same-as-before>
It will automatically show up in the PR on the github page.
If these are small changes they can be squashed together at the merge time and appear as a single commit in the repository.

Merging a PR

This is done by the project maintainers so nothing more for you to do, you’ll see a notification on github when the PR gets merged and closed.

4 Likes

Fastai-v2 - read this before posting please! 😊
(Jeremy Howard (Admin)) #2

@arora_aman if you haven’t tried it before, I suggest you take a look at this: https://github.com/github/hub

It makes life way easier for both beginners and experts!

2 Likes

(Aman Arora) #3

Thank you @jeremy

I made my first PR ever https://github.com/fastai/fastai_dev/pull/174 using hub but I think I still messed it up :slight_smile:

Can I please request you to make the thread a wiki so that anyone can edit and we can have a complete list of steps for everyone to follow including myself :slight_smile:

4 Likes

#4

I am polishing the scripts but whenever anyone submit a PR, it will be expected that the local library and notebooks match. If you made a change to the notebooks in one of the exported cells, you can export it to the library with notebook2script.py, if you made a change to the library, you can export it back to the notebooks with script2notebook.py.

The script diff_nb_script.py can let you know if there is a difference (and will be included in the CI early next week).

2 Likes

(Jeremy Howard (Admin)) #5

Done.

1 Like

(Jeremy Howard (Admin)) #6

Also the CI isn’t checking for stripped nbs at the moment - there’s extra metadata in some PRs and nothing picking that up.

0 Likes

(Hiromi Suenaga) #7

I am not sure what CI/CD tools fastai is using, but GitHub Actions has a hook for on.pull_request where you can run a shell script and check for stripped notebook. I was trying to see if I can get that to run the tests described in README, but it doesn’t have GPU and not great for testing.

0 Likes

(Jeremy Howard (Admin)) #8

@hiromi we use Azure Devops. We have things to check for stripping in most places, but it just hasn’t been set up for this folder yet.

0 Likes

#9

Update: the CI now automatically tests every commit/PR to check:

  • that all notebooks can be opened
  • that all notebooks are stripped from metadata (remember to run tools/run-after-git-clone onece after cloning the repo)
  • that there is no diff between the library in local and the result of an export in the notebooks.

If you changed something in one of the exported cells of one of the notebooks, either run the last cell of said notebook to export everything or run

python notebook2script.py

in the dev folder of the cloned repo.

If you changed something in one of the files in local that is generated by the notebooks, run

python script2notebook.py

in the dev folder of the cloned repo.
Note that this only works for small changes like a typo fix, if you want to add functions, use the notebooks.

If you want to check you have a clean diff, run

python diff_nb_script.py

in the dev folder of the cloned repo. If it doesn’t print anything, you are good to go.

4 Likes

(Slawek Biel) #10

@arora_aman Maybe add a section on updating a PR too? Like
git commit -m "improved the feature"
git push origin <enter-branch-name-same-as-before>

I think it’s also possible to do it without creating a new commit and instead git commit --amend the original one and pushing that.

1 Like

(Aman Arora) #11

@slawekbiel
Good idea! You should just be able to edit the post since its a Wiki thread :slight_smile:

0 Likes

(Jeremy Howard (Admin)) #12

It’s best when each separate PR has a separate commit history, otherwise it’s a bit harder to merge since the PR has a whole bunch of unrelated stuff in it.

1 Like

(Slawek Biel) #13

I’ve edited it again to clarify that you only create a fork once, rather than on every PR

1 Like

(Konstantin Dorichev) #14

Added these scripts’ description to the README.md

0 Likes