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

DOC: readthedocs #327

Merged
merged 19 commits into from
Apr 1, 2019
Merged

DOC: readthedocs #327

merged 19 commits into from
Apr 1, 2019

Conversation

mgxd
Copy link
Member

@mgxd mgxd commented Mar 27, 2019

Closes #256

This is the initial pass at a proper readthedocs page.

Doing this, I came to the realization our docstrings are severely lacking... something to keep in mind going forward. Ideally, I'd like to merge this in for the 0.5.4 release and improve on it as we go.

@mgxd mgxd added the doc label Mar 27, 2019
@mgxd mgxd requested review from yarikoptic and satra March 27, 2019 22:16
@codecov-io
Copy link

codecov-io commented Mar 28, 2019

Codecov Report

Merging #327 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #327   +/-   ##
=======================================
  Coverage   74.08%   74.08%           
=======================================
  Files          35       35           
  Lines        2643     2643           
=======================================
  Hits         1958     1958           
  Misses        685      685

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 f8d6b9a...d80a46f. Read the comment docs.

…-125-gc350b96e

It should generate markdown links as [LINKID][] to follow original markdown
(not github flavor) and so pandoc and possibly other tools do not barf
Copy link
Member

@yarikoptic yarikoptic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like a great start, thank you @mgxd !
I have pushed a minor tune up for the changelog.md markdown linking, and otherwise - let's merge and fixup/improve on it. One of the issues I believe remains docker tagging as we have discussed in #315 (comment)

# The short X.Y version
version = ''
# The full version, including alpha/beta/rc tags
release = '0.5.4'
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we better automate for this somehow I guess

can visit `our page on Docker Hub <https://hub.docker.com/r/nipy/heudiconv/tags>`_
to view available releases. To pull the latest release, run::

$ docker pull nipy/heudiconv:0.5.4
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we would be doomed to adjust it for each release, thus forgetting about it etc. We should return/add :latest IMHO.

DCMDIR=${DCMDIRS[${SLURM_ARRAY_TASK_ID}]}
echo Submitted directory: ${DCMDIR}

IMG="/singularity-images/heudiconv-0.5.4-dev.sif"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yet another version hardcoded....

in=CHANGELOG.md

# Replace them with Markdown references
sed -i -e 's/(\(#[0-9]\+\))/([\1])/g' "$in"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

as recently discovered, for "proper" original markdown, so we could convert using pandoc into .rst, should be links such as [#XXX][] not just [#XXX]. I will push a fixup directly into this branch

OUTPUT=/converted/data/voice

# find all DICOM directories that start with "voice"
DCMDIRS=(`find ${DCMROOT} -maxdepth 1 -name voice* -type d`)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

would it all work in there is a space in the path?

Eventually we should reincarnate ./heudiconv/cli/monitor.py development and absorb all the needed logic in there or in another supplementary helper command.

@yarikoptic
Copy link
Member

After "tests" pass, I will merge, resolve conflicts in #315, and push there as well

@yarikoptic yarikoptic merged commit 3467341 into master Apr 1, 2019
@yarikoptic yarikoptic deleted the doc/rtd branch November 4, 2022 00:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants