The contributor guide by nanjekyejoannah · Pull Request #1597 · geomstats/geomstats

nanjekyejoannah · 2022-06-28T09:28:40Z

This is the first draft for the contribution guide, whose issue is tracked here #1574

Dont review until we have had our meeting with @LPereira95 tommorrow.

I want to clarify about:

Tests
Templates for PRs and issues and
Benchmarks and
Triaging

nanjekyejoannah · 2022-06-29T14:23:10Z

Create the issue template and include it in the docs.

nanjekyejoannah · 2022-06-29T14:26:04Z

Benchmarks stability still under review, we will hold until we find a way of documenting how to run them. We will chat with sateja

nanjekyejoannah · 2022-07-19T12:38:09Z

This PR is ready for review.

adele-myers

This all is great! 😄 Thank you for all your hard work! I have made a few comments and suggestions.

adele-myers · 2022-07-19T15:39:26Z

-Code is not the only way to help the project:
+The instrcutions in this section detail the step-by-step
+process on setting up your development environment before
+contribution. One can also us it as a development checklist


line 17: us -> use

adele-myers · 2022-07-19T15:45:35Z

+6. At this point you should have the Geomstats code on your macine ready for development.
+To continue development, create a new development branch:
+
+    $ git checkout -b feature-branch


This comment isn't very important, but if we are making this tutorial for people who are very new to git, it might be helpful to change feature_branch to your-branch-name or something like that. When i am reading through tutorials, i find it helpful when people make it super obvious where i am supposed to add my own "name" or something like that. But again, this is not very important.

We have a different section for git intro, check dev setup where I use your-branch-name, this is about working on a new feature. Please look with this context and advise if we should still make this change.

adele-myers · 2022-07-19T15:48:24Z

+
+1. Using your browser, go to https://github.com and create an account if you dont have one.
+2. While there, navigate to https://github.com/geomstats/geomstats.
+3. [Fork](https://docs.github.com/en/get-started/quickstart/fork-a-repo)


It might be better to split up this sentence to make it clearer: "Fork the repository so that you will have your own copy. You can do this by clicking a button in the top righthand corner of the git webpage..."

The new commit has this change

adele-myers · 2022-07-19T15:51:33Z

+
+    $ git checkout -b feature-branch
+
+7. Verify tat you are on the new branch:


line 67 typo: tat -> that

adele-myers · 2022-07-19T15:52:10Z

+---------------------------------------
+
+We recommend using the conda virtual environment to create separation between your development environment
+from aany other Geomstats versions installed on your system. Create a virtual environment with conda using:


line 86 typo: aany -> any

adele-myers · 2022-07-19T16:27:59Z

+
+use the syntax::
+
+   my_function_with_a_very_long_name(


haha I laughed at "my_function_with_a_very_long_name" 🤣 .

adele-myers · 2022-07-19T16:28:40Z

+of the function is changed.
+
+These guidelines can be revised and modified at any time, the only constraint
+is that they should remain consistent through the codebase. To change geomstats


geomstats -> Geomstats' (with apostrophe at the end)

julesofthefields

This looks super helpful! Thank you, I have left some comments (mostly unimportant typos I think)

julesofthefields · 2022-07-19T17:07:50Z

- working on the website and improving the documentation,
- reference the project in your articles,
- or simply star it to say "I use it",
+The instrcutions in this section detail the step-by-step


instructions

julesofthefields · 2022-07-19T17:09:09Z

+from the project GitHub repository using the following instructions:

-.. _filing_bugs:
+#. Using your browser, go to `github.com <https://github.com>`_ and create an account if you dont have one.


julesofthefields · 2022-07-19T17:11:47Z

+
+    .. code-block:: bash  
+
+      $ pip install geomstats[<backend_name>]


maybe add an example (just so people do not literally write <backend_name> (I know I could have several years ago)) but this is really unimportant

I think its common convention to use fillers like these especially if we have many option. Inclined to keep as is.

I agree with @nanjekyejoannah.

julesofthefields · 2022-07-19T17:17:18Z

+
+    $ numpy tests
+
+Run a test file:


There are some test files that you can run with pytest checking whether or not the associated scripts do not raise any error. For example if you run test_hypersphere with pytest it will test that hypersphere runs properly (that everything works if the test file is well-written) I hope this makes sense. This is explained later on I think but maybe it would make sense to put it before?

julesofthefields · 2022-07-19T17:18:28Z


-In the first few steps, we explain how to locally install geomstats, and
-how to set up your git repository:
+Use an `asert` statement to check that the function under test returns


assert I assume?

luisfpereira

Thanks @nanjekyejoannah!

Very nice contribution. Looking forward for the continuation of this work.

luisfpereira · 2022-07-20T13:36:42Z

+.. note::
+   For more basic information about the `git` command line client, refer to 
+   this `detailed tutorial <https://docs.github.com/en/get-started/using-git/about-git>`_.
+   You can also checkout other GUI options like `tortoise git <https://tortoisegit.org/>`_ 


What about providing a list of guis instead of specifying? e.g. https://git-scm.com/downloads/guis

luisfpereira · 2022-07-20T13:44:02Z


-How to Write a Good Bug Report
-==============================
+#. | Fork the repository, obtaining your own copy. You can do this  using a button at the top right corner on github, under your username:


"[...] under you username. The following link will become available then:"

Something like this? otherwise I think there's lack of continuity in the sentence.

luisfpereira · 2022-07-20T13:51:55Z


-Contributing: Adding a new geometry or manifold
-----------------------------------------------
+      $ git checkout -b feature-branch


maybe: <branch-name>?

luisfpereira · 2022-07-20T16:27:27Z

+9. Use double quotes " and not single quotes ' for strings.
+
+10. If you need several lines for a function call,


this guidelines may have became outdated with the use of black.

I suggest we handle this in another PR, to avoid scope creep.

luisfpereira · 2022-07-20T16:27:59Z


 Building the documentation requires installing sphinx::

   pip3 install sphinx


luisfpereira · 2022-07-20T16:28:25Z

-To build the documentation, you need to be in the main ``geomstats`` folder. You can do this with::
-
-   sphinx-build docs/ docs/html
+To build the documentation, follow the steps discussed in `build the docs`_ to install other dependecies 


dependencies

luisfpereira · 2022-07-20T16:29:42Z

-==================
+-------------------
+
+**Intro to Docstrings**


Add the rendering of a meaningful example, as we've discussed.

luisfpereira · 2022-07-20T16:30:10Z

+Reporting bugs and features 
+===========================
+
+Sharing bugs and potential new features for the geomstats project is an equally significant conteibution.


contribution

How do we differentiate the other section of workflow of a contribution? I am inclined to name this something similar to bugs/features etc

nanjekyejoannah · 2022-07-28T15:47:44Z

I have addressed the requested changes from @LPereira95 , for what I havent handled, i left a comment.

luisfpereira · 2022-08-01T14:10:33Z

Thanks @nanjekyejoannah! I've made small changes to your PR.

I'll merge it now.

We can proceed this part by splitting the contributing guide in sections, as discussed, and then going deeper in the important sections (read tests, backends and docstrings).

First draft for the contributor guide

cb36782

nanjekyejoannah changed the title ~~WIP: The contributor guide~~ The contributor guide Jun 28, 2022

luisfpereira marked this pull request as draft July 1, 2022 11:59

This was referenced Jul 11, 2022

Template for the issue tracker #1603

Merged

Fix inheritance of NFold and ProductManifold (Issue #1588) #1604

Closed

Add contributor guide

0eebf9a

adele-myers reviewed Jul 19, 2022

View reviewed changes

luisfpereira marked this pull request as ready for review July 19, 2022 16:46

luisfpereira self-requested a review July 19, 2022 16:46

julesofthefields reviewed Jul 19, 2022

View reviewed changes

Update index.rst

0732994

luisfpereira requested changes Jul 20, 2022

View reviewed changes

Refactor the contribution guide

db82c15

Update contributing guide

bdb33b2

luisfpereira merged commit 9076d69 into geomstats:master Aug 1, 2022


		$ git checkout -b feature-branch

		7. Verify tat you are on the new branch:

		9. Use double quotes " and not single quotes ' for strings.

		10. If you need several lines for a function call,


		Building the documentation requires installing sphinx::

		pip3 install sphinx

Conversation

nanjekyejoannah commented Jun 28, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nanjekyejoannah commented Jun 29, 2022

Uh oh!

nanjekyejoannah commented Jun 29, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nanjekyejoannah commented Jul 19, 2022

Uh oh!

adele-myers left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

adele-myers Jul 19, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

julesofthefields left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

luisfpereira left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

nanjekyejoannah commented Jun 28, 2022 •

edited

Loading

nanjekyejoannah commented Jun 29, 2022 •

edited

Loading

adele-myers Jul 19, 2022 •

edited

Loading