Documentation Improvements #286

Cadair · 2017-11-29T11:45:24Z

closes #230

astrofrog

Looks good but have a few comments

astrofrog · 2017-11-29T19:30:56Z

docs/index.rst


 To start a package with the package template run::

-  cookiecutter gh:astropy/package-template
+  cookiecutter -c cookiecutter gh:astropy/package-template

 This will ask you a series of questions, and result in a directory inside your


Here I wouldn't say question as such - I would say it asks for values of settings (better worded) and then link again to the page that explains the settings.

astrofrog · 2017-11-29T19:31:30Z

docs/nextsteps.rst

+#########
+
+You should register your package on https://travis-ci.org and modify the
+``.travis.yml`` file to make the build pass. This will continuously test your


Can you clarify what kind of things need modifying? (give examples)

astrofrog · 2017-11-29T19:31:35Z

docs/nextsteps.rst

+automatically tested, so that you notice when something breaks. For further
+information see `here
+<https://github.com/astropy/astropy/wiki/Continuous-Integration>`__ and for
+lot's of example ``.travis.yml`` build configurations see `here


astrofrog · 2017-11-29T19:32:03Z

docs/nextsteps.rst

+Appveyor
+########
+
+Appveyor provides testing on the windows platform, if you want to enable this


windows -> Windows

astrofrog · 2017-11-29T19:32:21Z

docs/nextsteps.rst

+Coveralls
+#########
+
+If you register your package with coveralls.io, you will need to uncomment the


Add a sentence to say what coveralls is

astrofrog · 2017-11-29T19:33:28Z

docs/nextsteps.rst

+
+  MOCK_MODULES = ['<name of package to mock>', '<name of package to mock>']
+  for mod_name in MOCK_MODULES:
+      sys.modules[mod_name] = Mock()


I feel like this bit about mocking isn't really needed anymore now that dependencies can be installed with conda. Maybe better to just mention how to set up a conda environment file instead.

astrofrog · 2017-11-29T19:35:09Z

docs/updating.rst


   $ git checkout -b update_template

-3. **Copy the template over your package**: (replacing 'packagename' with your package name)::
+#. **Copy the template over your package**: (replacing 'packagename' with your package name)::

   $ cp -r /tmp/packagename ./


Wait but if you do that won't it make a folder with the name of the package inside the repo? Wouldn't it make more sense to do an rsync?

I changed this line to work (I tested it) and not use rsync because it's possible people wouldn't have that installed.

Cadair · 2017-12-20T15:30:08Z

@astrofrog @eteq opinons on this? With the objective of moving these docs out of core for 3.0?

astrofrog

Looks good! Super minor comments

astrofrog · 2018-02-09T14:34:19Z

docs/index.rst

+Template) and asking you a series of :ref:`questions <options>` to create a set
+of files and folders where your answers to the questions are pre-filled into the
+correct places. The package template uses this to allow you to specify things
+like your projects name as well as choose what parts and features of the


projects -> project's

astrofrog · 2018-02-09T14:35:23Z

docs/index.rst


-This will ask you a series of questions, and result in a directory inside your
-current working directory that has the name of your project.
+This prompt you for a series of :ref:`options<options>`,current working


space after comma

eteq

@Cadair - thanks for writing all this up - definitely a big step forward. Several inline comment, though.

In particular I'm concerned that if the user just follows these instructions as-is they might not really be forced to understand how the package files fit together. Some of the suggested additions I've given inline will hopefully help that? But it will be good to have someone test this once we've merged it and see if more explanation is needed.

eteq · 2018-02-09T22:13:33Z

docs/index.rst


-  pip install cookiecutter
+  pip install cookiecutter gitpython


I would swap the order of the conda and pip so that conda comes first. New users usually just do things in order, and if they are on conda it's safer for them to try that first.

eteq · 2018-02-09T22:15:30Z

docs/index.rst


-This will ask you a series of questions, and result in a directory inside your
-current working directory that has the name of your project.
+This prompt you for a series of :ref:`options<options>`, current working


Something's wrong here, like maybe a line got dropped? I'm not sure what this sentence is trying to convey...?

Also, maybe have this say . See the :ref:options<options> page for more details on the various options. Or something like that so it's clear how to get help?

eteq · 2018-02-09T22:27:58Z

docs/nextsteps.rst

+of the ``.travis.yml`` file can be found on the
+`Travis CI <https://docs.travis-ci.com/user/for-beginners/>`__ website.
+
+Continuous Integration services like Travis CI, continuously test your package


Maybe put this as the first paragraph of this section? since it's justifying why anyone would care about this section?

eteq · 2018-02-09T22:28:24Z

docs/nextsteps.rst

+Appveyor
+########
+
+Appveyor provides testing on the Windows platform, if you want to enable this


Maybe Appveyor provides testing -> Appveyor is similar to Travis, but provides testing ?

eteq · 2018-02-09T22:29:42Z

docs/nextsteps.rst

+Read the Docs
+#############
+
+If you want the documentation for your project to be hosted by `Read the Docs


Maybe add a sentence of motivation along the lines of "In addition to testing the code, it is often useful to build documentation continuously as the code is developed. Read the Docs is a web site that provides exactly this service." or something like that?

eteq · 2018-02-09T22:30:26Z

docs/nextsteps.rst

+following entries in "Advanced Settings" for your package on `Read the Docs
+<https://readthedocs.org>`_ should work:
+
+- Activate ``Install your project inside a virtualenv using setup.py install``


Instead of "activate", maybe "push the button that says"? I didn't understand what "Activate" meant for quite a while. (And similar below)

eteq · 2018-02-09T22:37:09Z

docs/options.rst

+#. ``use_read_the_docs``: If 'y' the ``read_the_docs.yml`` and ``.rtd-environment.yml`` files will be included for using conda on Read the Docs.
+#. ``sphinx_theme``: The value of the ``html_theme`` variable in the sphinx configuration file.
+#. ``initialize_git_repo``: If ``gitpython`` is installed this option will turn the rendered package into a git repository and add and initilize the ``astropy_helpers`` submodule.
+#. ``astropy_helpers_version``: The version number of the ``astropy_helpers`` submodule to be used, only used if ``initialize_git_repo`` is true.


I think this is missing something at the end about how to update these options later. Of course it's a bit complicated since some or more complex than others, but presumably it can say something like "many of the options are in setup.cfg, go there to modify things. The rest create various files."

eteq · 2018-02-09T22:40:16Z

docs/index.rst

-current working directory that has the name of your project.
+This prompt you for a series of :ref:`options<options>`, current working
+directory that has the name of your project. Once you have rendered the template
+read :ref:`next-steps`.


Before the "next steps" I think you need to add something along the lines of step 7 of http://docs.astropy.org/en/latest/development/astropy-package-template.html#managing-the-template-files-via-git . That is, the person using this has to go and look at the examples and understand and then remove them.

Remember that part of the purpose of this is to educate people who have never done this before. It's very important that part of the template process be maintained, even if this all makes it a lot easier to manually skip over that.

bsipocz

Quite a few, but mostly nitpick comments. Once the docs is updated I would like to check it by giving it someone else who is not in our ecosystem yet to try to set up a package using these instructions.

bsipocz · 2018-02-16T15:04:17Z

cookiecutter.json

@@ -18,7 +18,7 @@
    "use_read_the_docs": "y",
    "sphinx_theme": "astropy-boostrap",
    "initialize_git_repo": "y",
-    "astropy_helpers_version": "v2.0",
+    "astropy_helpers_version": "v2.0.2",


we may want to branch this to be v3.0 or 2.0.4 depending whether the users package is python3 only or not.

I am not sure how easy that would be to do. Could you open it as a different issue?

bsipocz · 2018-02-16T15:05:17Z

docs/index.rst

-The Astropy package template is designed to help quickly create new Python packages within the Astropy ecosystem.
+The Astropy package template is designed to help quickly create new Python
+packages within the Astropy ecosystem. This package mirrors the layout of the
+main `Astropy_` repository, as well as reusing much of the helper code used to


'astropy core library' rather than 'main Astropy repository'

bsipocz · 2018-02-16T15:06:01Z

docs/index.rst

+The Astropy package template is designed to help quickly create new Python
+packages within the Astropy ecosystem. This package mirrors the layout of the
+main `Astropy_` repository, as well as reusing much of the helper code used to
+organize `Astropy_`.


astropy and many Astropy affiliated packages.

bsipocz · 2018-02-16T15:07:06Z

docs/index.rst

-conda::
+<http://cookiecutter.readthedocs.io/>`_ project to make it easier to customise
+the template for your package. To use the package template you need cookiecutter
+installed. The package template also optionally makes use of 'gitpython' to


maybe link to gitpython, too?

bsipocz · 2018-02-16T15:10:01Z

docs/nextsteps.rst

+Next Steps
+==========
+
+Setting Up Continuous Integration


add (CI) to the end of the title

bsipocz · 2018-02-16T15:10:45Z

docs/nextsteps.rst

+
+Once you have rendered your package and set it up on `GitHub
+<https://github.com>`__ you may wish to enable Travis CI, Appveyor and Read the
+Docs (configuration for these services is included in the template, but you


maybe also like all of travis, appveyor and rtd here, too?

bsipocz · 2018-02-16T15:12:21Z

docs/nextsteps.rst

+Continuous Integration services like Travis CI, continuously test your package
+for each commit, even pull requests against your main repository will be
+automatically tested, so that you notice when something breaks. For further
+information see `here


I was told several times that linking on the word here is not good practice. Not sure though how to rephrase the sentence to be better...

bsipocz · 2018-02-16T15:14:30Z

docs/nextsteps.rst

+appveyor build is controlled by the ``appveyor.yml`` file included in the
+template (by default).
+
+Coveralls


maybe call it code coverage first? We're experimenting codecov, too, and on the long run may want to switch over? But changing the underlying service won't change the functionality and purpose of code coverage.

bsipocz · 2018-02-16T15:16:45Z

docs/options.rst

-15. ``use_appveyor_ci``: If 'y' the template will include an example ``appveyor.yml`` file for the Appveyor CI service.
-16. ``use_read_the_docs``: If 'y' the ``read_the_docs.yml`` and ``.rtd-environment.yml`` files will be included for using conda on Read the Docs.
-17. ``sphinx_theme``: The value of the ``html_theme`` variable in the sphinx configuration file.
+#. ``package_name``: This is a human readable name for your package, like 'Astropy' or 'SunPy'.


do we maybe have a better example where this name is not the same of module_name?

bsipocz · 2018-02-16T15:17:31Z

docs/options.rst

+#. ``long_description``: This is a multi-line description of your package.
+#. ``author_name``: The name or names of the authors.
+#. ``author_email``: A contact email for the authors.
+#. ``license``: The license of your project.


project --> package
(given that you use package everywhere above)

crawfordsm · 2018-03-23T20:53:00Z

@Cadair any chance you might have time to finished up this PR soon? I just started a new project last week and I didn't realize that this was working yet and used the old method. If this is now available, it would be great to advertise this. If you don't have time, I'd be happy to make a PR against your branch to take care of the edits if you'd be okay with that

Cadair · 2018-03-24T09:41:20Z

Hey @crawfordsm I don't think I am particularly likely to get to this in the next few weeks. Between GSOC, pre-pyastro things and my actual job it's not high up my list unfortunately.

If you have the time to clear it up please feel free to send PRs :)

crawfordsm · 2018-03-24T18:34:11Z

Okay I've made a PR against this -- I don't seem to have been able to merge directly into yours @Cadair so you'll have to merge my commits into this branch.

Gitpython docs

bsipocz · 2018-03-25T00:12:10Z

docs/nextsteps.rst

-Docs (configuration for these services is included in the template, but you
-could utilise others).
+`Continuous Integration 
+<https://github.com/astropy/astropy/wiki/Continuous-Integration>`__ (CI)


this is a very obsolate wiki page, what about linking it to wikipedia instead (and maybe to the https://en.wikipedia.org/wiki/Continuous_testing page as the integration seems slightly different, or actually the travis docs really nice and brief yet informative on this: https://docs.travis-ci.com/user/for-beginners/#What-is-Continuous-Integration-(CI)%3F).

offline approval

Cadair · 2018-03-28T10:06:38Z

Thanks a lot @crawfordsm !

bsipocz · 2018-03-28T22:26:48Z

Thanks @Cadair and @crawfordsm, it's great to see this merged.

A few of my comments stayed unaddressed while being collapsed by github. Anyway, they are minor things and can be done in a follow-up PR if any of us gets there.

Cadair mentioned this pull request Nov 29, 2017

remove the docs that have been moved to the package template astropy/astropy#6907

Merged

astrofrog suggested changes Nov 29, 2017

View reviewed changes

Cadair force-pushed the gitpython_docs branch from d344366 to 178a42c Compare November 29, 2017 20:06

Cadair added 6 commits November 29, 2017 20:06

document need to install gitpython

e7321a9

Extend the docs with relevant bits of core dev docs

2e343c5

more documentation improvements

38326ef

add a pacakge template reference

178a42c

refactor the sphinx theme if statement to make it easier to customise

1998efe

Update docs following @astrofrog's comments

555dfc4

astrofrog approved these changes Feb 9, 2018

View reviewed changes

Fix typos for @astrofrog

f2e6a9d

eteq previously requested changes Feb 9, 2018

View reviewed changes

Cadair added the cookiecutter label Feb 15, 2018

bsipocz reviewed Feb 16, 2018

View reviewed changes

crawfordsm added 3 commits March 24, 2018 14:27

edits to index

fe85c0c

edits to options

c97896d

edtis to nextsteps

c37a07b

crawfordsm mentioned this pull request Mar 24, 2018

Gitpython docs Cadair/package-template#1

Merged

Merge pull request #1 from crawfordsm/gitpython_docs

ca6997d

Gitpython docs

bsipocz reviewed Mar 25, 2018

View reviewed changes

crawfordsm merged commit e13e4fe into astropy:cookiecutter Mar 27, 2018

crawfordsm mentioned this pull request Mar 27, 2018

branches of astropy-helpers in cookiecoutter.json #303

Closed

Cadair deleted the gitpython_docs branch March 28, 2018 10:06

Documentation Improvements #286

Documentation Improvements #286

Conversation

Cadair commented Nov 29, 2017 • edited Loading

astrofrog left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Cadair commented Dec 20, 2017

astrofrog left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

eteq left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bsipocz left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

crawfordsm commented Mar 23, 2018

Cadair commented Mar 24, 2018

crawfordsm commented Mar 24, 2018

Choose a reason for hiding this comment

Cadair commented Mar 28, 2018

bsipocz commented Mar 28, 2018

Cadair commented Nov 29, 2017 •

edited

Loading