#39367 DOC: split contributing.rst
State: closed

- [x] closes #39367

I created a new PR due in an attempt to close #39367. Previously I had issues regarding environment setup but all seems to be working now thanks to Docker. I ran the linting tests prior to submitting this PR.


Labels:
name: Docs
name: Clean

Comments:
Could any of the project members assist me with resolving this PR? I am kind of at a loss as to how to resolve the last few checks. It seems to be a build issue but having checked the formatting and rst syntax all looks well to me. The new files I created followed the template of the existing files. I used a Docker container in VS Code to build the docs.
Thanks Marco, looking forward to resolving this issue for the project.
@David-dmh I think the issue may be here:
```
/home/runner/work/pandas/pandas/doc/source/development/contributing.rst:162: WARNING: undefined label: contributing.documentation
/home/runner/work/pandas/pandas/doc/source/development/contributing_codebase.rst:837: WARNING: undefined label: contributing.documentation
```
We're still referencing the contributing.documentation section, but it's been moved to its own file. Try referencing the new file similar to how `code_style.rst` is on line 685 of `contributing.rst` from master.

@MarcoGorelli does this seem right to you? I'm partially making an educated guess here as I'm not terribly familiar with Sphinx myself.
@David-dmh Looks like just a couple pre-commit errors (trailing whitespace and spelling) but otherwise green
@dsaxton ok good, I'm busy working on the pre-commit checks now.
I fixed the spelling however could not identify any trailing whitespaces. I used an extension in VS code to highlight and none came up for the files I added/edited. Is there perhaps a way of determining the specific file(s) or line(s) resulting in the check failing?
Hi @David-dmh - you can run `pre-commit` on the files you've modified, see https://pandas.pydata.org/pandas-docs/dev/development/contributing.html#pre-commit
With regards to fixing the last trailing whitespace error, I ran pre-commit for the entire doc/source/development dir however I passed the pre-commit checks. The issue seems to be arising from the unmodified portion of the project files. I will then need to run pre-commit elsewhere to isolate the issue.
Can you post the command you ran, along with output?
> With regards to fixing the last trailing whitespace error, I ran pre-commit for the entire doc/source/development dir however I passed the pre-commit checks. The issue seems to be arising from the unmodified portion of the project files. I will then need to run pre-commit elsewhere to isolate the issue.

Check out line 150 of `contributing.rst`, it looks like that is where the issue is
@David-dmh Don't worry about the other pre-commit failure, it's unrelated and there's already a fix for it
> @David-dmh Don't worry about the other pre-commit failure, it's unrelated and there's already a fix for it

Ok, so apart from the new proposals it looks good?
> Ok, so apart from the new proposals it looks good?

I think so, to be safe we probably want to wait for the CI fixes to be merged and then merge those in here
Thanks for updating - current CI failures are unrelated
Sorry for slow response. My thinking with "splitting out sub-pages for…setting up an environment outside of Docker" was to make the environment setup instructions for new contributors as streamlined and foolproof as possible. I believe directing them to Docker is the best way to do that. If others disagree, happy to take that conversation elsewhere to not hold up this pull request. Hope that makes sense!
> Sorry for slow response. My thinking with "splitting out sub-pages for…setting up an environment outside of Docker" was to make the environment setup instructions for new contributors as streamlined and foolproof as possible. I believe directing them to Docker is the best way to do that. If others disagree, happy to take that conversation elsewhere to not hold up this pull request. Hope that makes sense!

Perhaps keep this PR as is and then consider consolidating into a single file for setting up environment later? What do you say @MarcoGorelli?

I do think the idea that Docker is the easiest way to contribute is debatable. Some may see it as a lot of overhead compared to just using a virtual environment.
> I do think the idea that Docker is the easiest way to contribute is debatable

Yes - also, see [this comment](https://github.com/pandas-dev/pandas/pull/39936#issuecomment-782896088) by Jeff:

> I suppose this is ok, we don't normally recommend using docker at all, but to the extent people use it, having a script is good.

From my own experience of mentoring people at sprints using both Docker and Miniconda, the latter got them up-and-running faster, so I'd rather not keep the Docker instructions separate or make them look like the preferred approach

----

@David-dmh could you fetch and merge upstream/master please?

If you could also make `contributing_environment.rst` which includes _both_ Docker instructions as well as non-Docker, and remove `env_outside_docker`, that'd be great, else it can be a follow-up PR
> > I do think the idea that Docker is the easiest way to contribute is debatable
> 
> Yes - also, see [this comment](https://github.com/pandas-dev/pandas/pull/39936#issuecomment-782896088) by Jeff:
> 
> > I suppose this is ok, we don't normally recommend using docker at all, but to the extent people use it, having a script is good.
> 
> From my own experience of mentoring people at sprints using both Docker and Miniconda, the latter got them up-and-running faster, so I'd rather not keep the Docker instructions separate or make them look like the preferred approach
> 
> @David-dmh could you fetch and merge upstream/master please?
> 
> If you could also make `contributing_environment.rst` which includes _both_ Docker instructions as well as non-Docker, and remove `env_outside_docker`, that'd be great, else it can be a follow-up PR

Ok will do so tomorrow
Queued now, I just merged. 
With regards to these 2 errors, are they also unrelated? Both with bash exit code 125.
> With regards to these 2 errors, are they also unrelated? Both with bash exit code 125.

Very likely unrelated. It looks like we still need to consolidate into `contributing_environment.rst` like @MarcoGorelli recommended.
> > With regards to these 2 errors, are they also unrelated? Both with bash exit code 125.
> 
> Very likely unrelated. It looks like we still need to consolidate into `contributing_environment.rst` like @MarcoGorelli recommended.

Ok, is the consensus to open a new PR for that or rather to do it here?
That's OK with me, thanks @David-dmh - could you create an issue for creating the environment.rst file it so it doesn't get forgotten please?

I likely won't have time to run this this week, so @dsaxton if it looks good to you don't wait for me to merge
Sorry for my late response, the conflict should be resolved now. I have drafted a new `contributing_environment.rst` file and hope to finalize soon. So I will commit to `new-feature` and address here. Will also be sure to delete the old file.
The new file will have the following headers and sub-headers:
* Creating an environment using Docker
* Creating an environment without Docker
    * Installing a C compiler
    * Creating a Python environment
    * Creating a Python environment (pip)
Sounds great, thanks! :+1: ping when ready
@David-dmh Looks like a lot of other files got edited (this can happen if a commit happens on GitHub and then your local branch diverges). One way you can fix this is to rebase against `upstream/master` and then force push, which will probably require a few steps:
```
git checkout new-feature
git fetch upstream
git rebase upstream/master
```
You'll likely get a conflict that looks like this:
```
CONFLICT (content): Merge conflict in doc/source/development/contributing.rst
```
For whichever file it shows as having a conflict you can resolve this way:
```
git checkout --ours doc/source/development/contributing.rst
git add doc/source/development/contributing.rst
git rebase --continue
```
You may have to do this a couple times until finally the rebase works, then you can do
```
git push -f
```
which should get things back into a good state.
> @David-dmh Looks like a lot of other files got edited (this can happen if a commit happens on GitHub and then your local branch diverges). One way you can fix this is to rebase against `upstream/master` and then force push, which will probably require a few steps:
> 
> ```
> git checkout new-feature
> git fetch upstream
> git rebase upstream/master
> ```
> 
> You'll likely get a conflict that looks like this:
> 
> ```
> CONFLICT (content): Merge conflict in doc/source/development/contributing.rst
> ```
> 
> For whichever file it shows as having a conflict you can resolve this way:
> 
> ```
> git checkout --ours doc/source/development/contributing.rst
> git add doc/source/development/contributing.rst
> git rebase --continue
> ```
> 
> You may have to do this a couple times until finally the rebase works, then you can do
> 
> ```
> git push -f
> ```
> 
> which should get things back into a good state.

Thanks for this @dsaxton, I did the process twice which finished the rebase then pushed, will await checks.
It seems like the issue mentioned by @MarcoGorelli in `contributing_environment.rst` was not resolved (for example it still displays the old Python version). How can this be done? I thought this process would integrate the corresponding changes.
> It seems like the issue mentioned by @MarcoGorelli in `contributing_environment.rst` was not resolved (for example it still displays the old Python version). How can this be done? I thought this process would integrate the corresponding changes.

Most likely stale text was copied into the new file when you created it, in which case git has no way of knowing the content should be different. You'll need to apply that change as a new commit.
> > It seems like the issue mentioned by @MarcoGorelli in `contributing_environment.rst` was not resolved (for example it still displays the old Python version). How can this be done? I thought this process would integrate the corresponding changes.
> 
> Most likely stale text was copied into the new file when you created it, in which case git has no way of knowing the content should be different. You'll need to apply that change as a new commit.

Thanks @dsaxton this makes sense. I pushed again to address this issue. If I am correct, it seems all exit codes point to test failures on files I did not modify apart from pulling new changes. Again, thanks for your patience helping me resolve these simple issues I am facing.
> Thanks for updating - I tried building this but got
> 
> ```
> /home/marco/pandas-marco/doc/source/getting_started/install.rst:201: WARNING: undefined label: contributing.dev_env 
> ```
> 
> which seems relevant as you've removed that label
> 
> Other than that, this looks ready to me!

Ok good, just let me know if there is anything else required to wrap it up.