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.

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

Improve docs #8

Open
inati opened this issue Oct 9, 2015 · 4 comments
Open

Improve docs #8

inati opened this issue Oct 9, 2015 · 4 comments

Comments

@inati
Copy link
Contributor

inati commented Oct 9, 2015

This is a placeholder issue for a discussion around how to improve the documentation, webpage, automated tests, etc., following up on the discussion that came out of #7. I'm sure this will apply to https://github.com/ismrmrd/ismrmrd-python as well.

Does having this be a sci-kit make sense? At the very least we should look at how those projects are organized. Some of them are very well put together (see http://scikit-image.org and http://scikit-learn.org for example). There is also an example kit has a lot of the infrastructure already laid out (https://scikits.appspot.com/example).

At the very least, we should have a build and test every time there is a commit to master and an upload to pypi and a rebuild of the webpages/docs every time we tag a release. We have some of this already for the ismrmrd-python piece.

@inati
Copy link
Contributor Author

inati commented Oct 9, 2015

@grlee77 and @naegelejd your input here would be most welcome.

@grlee77
Copy link
Contributor

grlee77 commented Oct 9, 2015

I don't think this necessarily needs to be a scikit, but I don't have a strong opinion either way. Perhaps if it was one, it might draw additional users/contributers via the links from the scipy website.

I periodically contribute to scikit-image, scikit-cuda and scipy and am also a developer on PyWavelets.

These projects use Travis to run test suites against multiple python versions. This is free for open-source projects.

The tests themselves are written using nose as detailed here:
https://github.com/numpy/numpy/blob/master/doc/TESTS.rst.txt

They follow PEP8 coding style conventions and use the numpy docstring format: https://github.com/numpy/numpy/blob/master/doc/HOWTO_DOCUMENT.rst.txt

There is also often attempt to standardize the format of commit messages by adding a short descriptor such as BUG: , ENH:, TEST:, etc at the start of each commit as detailed here:
http://docs.scipy.org/doc/numpy/dev/gitwash/development_workflow.html

I am very familiar with the PEP8 style guidelines and have experience writing tests for the packages mentioned above. I know less about configuring Travis, but it should be relatively easy to follow the examples provided by existing scikits once there is a test suite in place.

I would suggest the following steps in sequence:

  1. update to numpy style docstrings and improve PEP8 compliance. I can submit a PR for this soon if desired.
  2. create a tests subfolder within ismrmrdtools and start adding tests, with separate files for each module: e.g. test_coils.py, test_grappa.py, etc..
  3. once there is a reasonable level of test coverage, setup Travis for continuous-integration

@grlee77
Copy link
Contributor

grlee77 commented Oct 9, 2015

two other suggestions:
1.) make an examples folder at the top level and move csm_estimation_demo.py, etc there. This will keep things cleaner, especially once additional examples are added.
2.) add LICENSE information. copy the license from gadgetron?

@inati
Copy link
Contributor Author

inati commented Oct 9, 2015

All of the above sounds very reasonable.

I looked at scikit-image and it looks like that project uses travis to build its docs after a successful test and pushes to github pages (scikit-image.org points to sickout-image.github.io), scikit-learn seems to do the same. @naegelejd almost got this scheme working for ismrmrd (I think the hangup was the github token but I don't recall exactly) - the ismrmrd c++ library itself is built and tested on travis, but the docs are not being auto generated. ismrmrd.github.io is a separate repo.

For reference, here are reminders for myself:
https://github.com/scikit-image/scikit-image/blob/master/.travis.yml
https://github.com/scikit-image/scikit-image/blob/master/tools/deploy_docs.sh

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants