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

Dev.ej/wizard navigation #561

Open
wants to merge 16 commits into
base: main
Choose a base branch
from
Open

Dev.ej/wizard navigation #561

wants to merge 16 commits into from

Conversation

joanise
Copy link
Member

@joanise joanise commented Sep 26, 2024

PR Goal?

Add several navigation features to the EveryVoice new-project wizard:

  • The ability to go back to a previous step (use Ctrl-C, option 1)
  • The ability to view the question tree (Ctrl-C, option 3, with hidden option --trace to auto display it at each step)
  • Hidden option --debug-state displays the current state before each question
  • The ability to save progress (Ctrl-C, option 4), and the --resume-from option to resume from a saved state

Fixes?

Fixes #545
Fixes #468
Fixes #546

Feedback sought?

Code review, and playing around with the wizard to see how it feels, how it works, improvements you might like to see.

Priority?

beta

Tests added?

Not yet - I'm making this draft PR to seek feedback but I'll add unit testing before finalizing it.

In progress: navigation has unit testing now.

How to test?

Run the wizard with the new options, hit Ctrl-C are various points during the wizard and play with the options.

Confidence?

medium-high, since I have tested it interactively quite extensively, but will be higher once unit testing is in place.

Version change?

might have warranted a minor version bump since it's new features, but we're preparing the next alpha so that doesn't make sense. Nothing broken, in any case, so no major bump.

Related PRs?

nope, it's all in EV itself.

Using a custom NodeMixinWithNavigation with prev() and next() method
will enable going back through the tree during the wizard.
Fixes formatting in many places.
Adds nice highlighting in some.
But, do not redefine builtin print, use rich_print to be clear everywhere.

Fixes #546: question titles being truncated instead of wrapped.
Copy link

semanticdiff-com bot commented Sep 26, 2024

Review changes with SemanticDiff.

Analyzed 17 of 18 files.

Overall, the semantic diff is 9% smaller than the GitHub diff.

Filename Status
✔️ everyvoice/cli.py 26.01% smaller
✔️ everyvoice/run_tests.py Analyzed
✔️ everyvoice/wizard/__init__.py 8.77% smaller
✔️ everyvoice/wizard/basic.py Analyzed
✔️ everyvoice/wizard/dataset.py 9.9% smaller
✔️ everyvoice/wizard/main_tour.py 2.27% smaller
✔️ everyvoice/wizard/prompts.py 9.41% smaller
✔️ everyvoice/wizard/simple_term_menu_win_stub.py 26.55% smaller
✔️ everyvoice/wizard/utils.py 0.05% smaller
✔️ everyvoice/wizard/validators.py 72.25% smaller
✔️ everyvoice/tests/preprocessed_audio_fixture.py 37.34% smaller
✔️ everyvoice/tests/stubs.py 27.31% smaller
✔️ everyvoice/tests/test_cli.py 2.62% smaller
✔️ everyvoice/tests/test_preprocessing.py 82.18% smaller
✔️ everyvoice/tests/test_utils.py Analyzed
✔️ everyvoice/tests/test_wizard.py 1.61% smaller
✔️ everyvoice/tests/test_wizard_helpers.py Analyzed
everyvoice/tests/data/metadata.psv Unsupported file format

@joanise joanise marked this pull request as draft September 26, 2024 20:13
Copy link
Contributor

github-actions bot commented Sep 26, 2024

CLI load time: 0:00.29
Pull Request HEAD: e22f6e85185c27bab81c0217e43d23075ec7d60c
Imports that take more than 0.1 s:
import time: self [us] | cumulative | imported package

Copy link

codecov bot commented Sep 26, 2024

Codecov Report

Attention: Patch coverage is 93.95770% with 20 lines in your changes missing coverage. Please review.

Project coverage is 76.15%. Comparing base (d7fbb81) to head (e22f6e8).

Files with missing lines Patch % Lines
everyvoice/wizard/__init__.py 90.24% 10 Missing and 6 partials ⚠️
everyvoice/wizard/dataset.py 96.42% 2 Missing and 1 partial ⚠️
everyvoice/cli.py 50.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #561      +/-   ##
==========================================
+ Coverage   74.87%   76.15%   +1.28%     
==========================================
  Files          46       46              
  Lines        3144     3372     +228     
  Branches      510      550      +40     
==========================================
+ Hits         2354     2568     +214     
- Misses        691      700       +9     
- Partials       99      104       +5     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@joanise joanise force-pushed the dev.ej/wizard-navigation branch 3 times, most recently from 938ffd4 to f6a405b Compare September 27, 2024 15:26
@joanise joanise marked this pull request as ready for review October 1, 2024 15:02
…nd Step

Plus some other tydying of wizard steps and docs
Required changing the concise @contextmanager functions into more
verbose classes which can be entered in more than one `with` statement.
Less elegant, sigh, but it's what I needed to test back() in a simple
way.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
1 participant