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

DOC: Fix reStructuredText formatting issues in coding style guideline. #1421

Merged
merged 1 commit into from Feb 13, 2018

Conversation

Projects
None yet
3 participants
@jhlegarreta
Contributor

jhlegarreta commented Feb 10, 2018

Fix RST markup issues in the coding_style_guideline.rst file.

Fixes #1390.

@jhlegarreta

This comment has been minimized.

Contributor

jhlegarreta commented Feb 10, 2018

@jchoude Thanks for reporting the issue !

Rather than checking the GitHub file, it looks like checking the dipy website version gives a more intuitive idea of what's going on.

The mission file seems to have issues as well, but it's rendered correctly.

Please, correct me if I'm wrong, and if both versions should be rendered correctly.

@codecov-io

This comment has been minimized.

codecov-io commented Feb 10, 2018

Codecov Report

Merging #1421 into master will decrease coverage by <.01%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1421      +/-   ##
==========================================
- Coverage   87.39%   87.38%   -0.01%     
==========================================
  Files         237      237              
  Lines       30069    30069              
  Branches     3232     3232              
==========================================
- Hits        26278    26277       -1     
  Misses       3043     3043              
- Partials      748      749       +1
Impacted Files Coverage Δ
dipy/core/graph.py 73.8% <0%> (-1.2%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 666d783...e332c38. Read the comment docs.

@skoudoro

Thanks for doing this @jhlegarreta !

Can I request some additional change?

  • Can you add release 0.12 and 0.13 on this page
  • Can you replace HBM 2015by ÒHBM 2015 on this page
object called ``GradientTable`` which holds all the acquisition specific
parameters, e.g. b-values, b-vectors, timings and others.*
* Cite the relevant papers. Use the *[NameYear]* convention for
cross-referencing them, such as in [Garyfaillidis2014]_, and put them

This comment has been minimized.

@skoudoro

skoudoro Feb 10, 2018

Member

Garyfallidisinstead of Garyfaillidis

This comment has been minimized.

@jhlegarreta

jhlegarreta Feb 11, 2018

Contributor

Thanks ! I inspected this visually yesterday without being able to tell why it was not being linked 😬

DIPY object, etc.).
* When referring to relative paths, use the backquote inline markup
convention, such as in ``doc/devel``. Do not add the
greater-than/less-than signs to enclose the path.
"""

This comment has been minimized.

@skoudoro

skoudoro Feb 10, 2018

Member

Do we really need this quotes? I think you can remove them because they look strange as you can see on this page

This comment has been minimized.

@jhlegarreta

jhlegarreta Feb 11, 2018

Contributor

I think they make easier reading the text, standing out what is a code snippet or any other construct that belong to the structure of the toolkit. The reason why they are double inverted quotes instead of simple inverted quotes is due to the Sphinx syntax in the docstrings (if I remember well).

A question relative to the way dirs had to be described was raised in a comment #1314 .

This comment has been minimized.

@skoudoro

skoudoro Feb 12, 2018

Member

I think we are not talking about the same line. I am talking about the double quotes here, line 113

This comment has been minimized.

@jhlegarreta

jhlegarreta Feb 12, 2018

Contributor

OK, now I got it. Sorry. My bad.

@@ -117,4 +123,5 @@ References
<http://journal.frontiersin.org/Journal/10.3389/fninf.2014.00008/abstract>`_
Frontiers in Neuroinformatics, vol.8, no.8.
.. include:: ../links_names.inc
"""

This comment has been minimized.

@skoudoro

skoudoro Feb 10, 2018

Member

same comment as above

This comment has been minimized.

@skoudoro

skoudoro Feb 12, 2018

Member

Same comment as above. The double quotes are not necessary here.

@skoudoro

This comment has been minimized.

Member

skoudoro commented Feb 10, 2018

Concerning the rendering, I wonder why GitHub does not take in consideration link_names.inc

@jhlegarreta jhlegarreta force-pushed the jhlegarreta:FixCodingStyleGuidelineFormatting branch from bbd4260 to 6ec74bc Feb 11, 2018

@jhlegarreta

This comment has been minimized.

Contributor

jhlegarreta commented Feb 11, 2018

Yes, I also realized that GitHub does not link the cross-refs in link_names.inc but have not investigated 😟

@skoudoro

This comment has been minimized.

Member

skoudoro commented Feb 12, 2018

It seems that github don't allow the include directive for security reason (look here).

@jhlegarreta jhlegarreta force-pushed the jhlegarreta:FixCodingStyleGuidelineFormatting branch from 6ec74bc to 244a049 Feb 12, 2018

@jhlegarreta

This comment has been minimized.

Contributor

jhlegarreta commented Feb 12, 2018

OK. Fixed the quotes. Thanks for the explanation on the include directive issue @skoudoro

@skoudoro

This comment has been minimized.

Member

skoudoro commented Feb 12, 2018

  • Can you add release 0.12 and 0.13 on this page
  • Can you replace HBM 2015by ÒHBM 2015 on this page

It will be nice if you can fix these additional issues on this PR. Thank you !

@jhlegarreta jhlegarreta force-pushed the jhlegarreta:FixCodingStyleGuidelineFormatting branch from 244a049 to e332c38 Feb 12, 2018

@jhlegarreta

This comment has been minimized.

Contributor

jhlegarreta commented Feb 12, 2018

Sorry Serge. I missed that in your first comment. Done.

Jon Haitz Legarreta
DOC: Fix reStructuredText formatting issues in coding style guideline.
Fix RST markup issues in the `coding_style_guideline.rst` file.

Fixes #1390.
@skoudoro

This comment has been minimized.

Member

skoudoro commented Feb 13, 2018

Thank you @jhlegarreta, it looks better now after test.

merging !

@skoudoro skoudoro merged commit 7600480 into nipy:master Feb 13, 2018

2 of 3 checks passed

codecov/project 87.38% (-0.01%) compared to 666d783
Details
codecov/patch Coverage not affected when comparing 666d783...e332c38
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@jhlegarreta jhlegarreta deleted the jhlegarreta:FixCodingStyleGuidelineFormatting branch Feb 14, 2018

ShreyasFadnavis pushed a commit to ShreyasFadnavis/dipy that referenced this pull request Sep 20, 2018

Merge pull request nipy#1421 from jhlegarreta/FixCodingStyleGuideline…
…Formatting

DOC: Fix reStructuredText formatting issues in coding style guideline.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment