Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add a Contributor Guide #42

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

Add a Contributor Guide #42

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 34 additions & 0 deletions CONTRIBUTING.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
# Contributor Guide

## Environment setup and workflow
- Clone the repo
- Create a fresh python environemt with `.binder/requirements.txt` file
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bsipocz I noticed you recently moved requirements.txt to .binder/ from tutorials/. Can we keep it in tutorials/ and create a symlink to it in .binder/?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Symlinks notoriously won't work on windows, at least I had issues with keeping symlinks in repos before (and we only symlinked licences and contributing guides rather than actually important content)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought users wouldn't need to touch this requirements.txt file all since notebooks in this repo should always pip install what they need in the first code cell -- I thought this file was only here for the rendering? I do see that the instructions below have some dependency on in it (i.e., assuming jupytext extension is installed). Since my workflow with notebooks is so different I don't know exactly what the user will need, so I'm fine with whatever's easiest (either telling them to install that extension separately or just pointing them to the requirements.txt file even though it isn't intended for them). Although, the other repo doesn't have a requirements.txt, so if this file gets moved there that's another factor to think about.

- Open `jupyter lab` or `jupyter notebook` (so that jupytext extension can take effect)
- Then right-click on a MyST markdown notebook, choose `Open With` -> `Notebook`
or `Jupytext Notebook` (more info [here](https://nasa-navo.github.io/navo-workshop/00_SETUP.html#handling-notebooks-in-myst-markdown-format)).
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we should link more upstream than NAVO, e.g. numpy

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you share the link? Last I checked numpy docs, I got confused because they were instructing to "pairing the notebook and md" but then I learned from you that that's not required.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I knew it would bite me back, we need to cleanup the numpy docs first, 😅 : numpy/numpy-tutorials#186


> Note: If you'd like to work with this repository in an editor/IDE of your choice,
make sure that you have relevant extensions to parse MyST markdown notebooks to
interactive python notebook (ipynb) and to reflect the changes made there to markdown
format:
>- VSCode: doesn't seem to have a reliable extension to open md files as notebook, so you may have to manually pair them using jupytext in terminal.
>- Other editors: ...

## Updating an existing notebook (.md format)
- Open it in jupyter notebook or lab, jupytext extension should be installed (it's already listed in `requirements.txt` file mentioned above)
- Right click on a md notebook file and "open with" notebook format.
- Edit it as you would a regular notebook and once saved the changes should
reflect in md
- Make sure only content changes appear in md diff, not the frontmatter, before
comitting
- Make a PR once ready (TODO: explain how like forking etc.)

## Adding a new notebook (from .ipynb format)
To add an ipynb notebook you were working on to this repository, convert that to
MyST markdown using:

```python
jupytext --to md:myst notebook.ipynb
Comment on lines +30 to +31
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
```python
jupytext --to md:myst notebook.ipynb
```sh
jupytext --to md:myst notebook.ipynb

```

Then add it to `tutorials/` and also add its path in a relevant TOC in `index.md`.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just flagging this line because there are similar instructions in the existing docs in the other repo, but the process is different in that repo. So we need to sync them up depending on which repo we chose.

Loading