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

Updates to the documentation to reflect the refactored CLI #93

Merged
merged 3 commits into from
Nov 22, 2021

Conversation

joanise
Copy link
Member

@joanise joanise commented Nov 17, 2021

The dev.cli branch makes a lot of changes to the CLI for readalongs. This PR updates docs/ to reflect all those changes.

It's meant to be fast-forward merged into dev.cli (via the git CLI, not via GitHub) once we're happy with it.

@codecov
Copy link

codecov bot commented Nov 17, 2021

Codecov Report

Merging #93 (67f0593) into dev.cli (52470e9) will not change coverage.
The diff coverage is n/a.

❗ Current head 67f0593 differs from pull request most recent head 1d90f10. Consider uploading reports for the commit 1d90f10 to get more accurate results
Impacted file tree graph

@@           Coverage Diff            @@
##           dev.cli      #93   +/-   ##
========================================
  Coverage    75.64%   75.64%           
========================================
  Files           26       26           
  Lines         1601     1601           
  Branches       284      284           
========================================
  Hits          1211     1211           
  Misses         347      347           
  Partials        43       43           
Impacted Files Coverage Δ
readalongs/cli.py 93.92% <ø> (ø)

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 52470e9...1d90f10. Read the comment docs.

Copy link
Collaborator

@roedoejet roedoejet left a comment

Choose a reason for hiding this comment

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

This is really great @joanise thanks! There was one type end instead of eng that should get fixed. Did you want me to fill in some of the todos or should we merge as is and work on those docs in a different PR?


## Build the documentation locally

To build the documention for local inspection, run one of these commands,
Copy link
Collaborator

Choose a reason for hiding this comment

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

I really like this section @joanise - great addition.

docs/cli-guide.rst Outdated Show resolved Hide resolved
docs/cli-guide.rst Outdated Show resolved Hide resolved

Silences are inserted in the audio stream wherever a ``silence`` element is
found in the XML input.
**TODO say something about how the silence placement determined.**
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do we want to merge this with the TODOs? Or did you put these here for me to fill out?

Copy link
Member Author

Choose a reason for hiding this comment

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

@roedoejet Actually, Marc has reported errors in where silences are inserted, especially when there are anchors next to silences. Short answer, yes, I'd like you to fill this in. I'll merge it into dev.cli as is, but it a TODO for you, in conjunction with fixing #89 and Marc's reported silence placement issues.

"The language code(s) for text in TEXTFILE (use only with -i, i.e., with plain text input); "
"multiple codes can be joined by ':' or by repeating the option; "
"The language code(s) for text in TEXTFILE (use only with plain text input); "
"multiple codes can be joined by ':', or by repeating the option, to enable the g2p cascade; "
Copy link
Collaborator

Choose a reason for hiding this comment

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

Link to g2p cascade docs here

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, that would be important. I'm not quite sure how best to do it...

I could link to readthedocs, but I don't really like that to be the main source. I'm inclined to put a paragraph about the g2p cascade in the def align docstring, so it's in the body of g2p align -h, refer to that in the -l switch doc for both align and prepare.

Hmm, I see I documented the cascade in readalongs g2p -h. I'll need to think this through more carefully and do something coherent in g2p, prepare and align... Maybe all I need is something like (run 'readalongs g2p -h' for details).

@@ -416,7 +416,7 @@ def align(**kwargs):
callback=joiner_callback(LANGS),
help=(
"The language code(s) for text in PLAINTEXTFILE; "
"multiple codes can be joined by ':' or by repeating the option; "
"multiple codes can be joined by ':', or by repeating the option, to enable the g2p cascade; "
Copy link
Collaborator

Choose a reason for hiding this comment

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

And link here too

@joanise joanise merged commit 1d90f10 into dev.cli Nov 22, 2021
@joanise joanise deleted the dev.cli-docs branch November 22, 2021 17:21
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

Successfully merging this pull request may close these issues.

2 participants