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.

Sign up for GitHub

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

Jump to bottom

Bug: error while use anki_to_source.yaml for [Extended] decks #666

Open
Joilence opened this issue Jan 16, 2025 · 3 comments · May be fixed by #668
Open

Bug: error while use anki_to_source.yaml for [Extended] decks #666

Joilence opened this issue Jan 16, 2025 · 3 comments · May be fixed by #668

Comments

@Joilence
Copy link

Joilence commented Jan 16, 2025 •
edited
Loading

Reproduce steps following CONTRIBUTING.md: Anki to Source, but use [EN] [Extended] deck. [EN] deck works fine.

Reason: [Extended] deck has model Ultimate Geography [Extended] instead of Ultimate Geography.

Error message:

❯ brain_brew run recipes/anki_to_source.yaml
...
(ignorable warning due to: https://github.com/Stvad/CrowdAnki/issues/62)
...

Traceback (most recent call last):
  File "~/localcode/anki-geo-ultimate-geography/.venv/bin/brain_brew", line 8, in <module>
    sys.exit(main())
  File "~/localcode/anki-geo-ultimate-geography/.venv/lib/python3.7/site-packages/brain_brew/main.py", line 19, in main
    command.execute()
  File "~/localcode/anki-geo-ultimate-geography/.venv/lib/python3.7/site-packages/brain_brew/commands/run_recipe/run_recipe.py", line 15, in execute
    recipe = TopLevelBuilder.parse_and_read(self.recipe_file_name, self.verify_only)
  File "~/localcode/anki-geo-ultimate-geography/.venv/lib/python3.7/site-packages/brain_brew/commands/run_recipe/top_level_builder.py", line 60, in parse_and_read
    return cls.from_list(recipe_data)
  File "~/localcode/anki-geo-ultimate-geography/.venv/lib/python3.7/site-packages/brain_brew/commands/run_recipe/recipe_builder.py", line 20, in from_list
    tasks = cls.read_tasks(data)
  File "~/localcode/anki-geo-ultimate-geography/.venv/lib/python3.7/site-packages/brain_brew/commands/run_recipe/recipe_builder.py", line 68, in read_tasks
    task_or_tasks = [matching_task.from_repr(task_arguments)]
  File "~/localcode/anki-geo-ultimate-geography/.venv/lib/python3.7/site-packages/brain_brew/commands/run_recipe/parts_builder.py", line 40, in from_repr
    return cls.from_list(data)
  File "~/localcode/anki-geo-ultimate-geography/.venv/lib/python3.7/site-packages/brain_brew/commands/run_recipe/recipe_builder.py", line 20, in from_list
    tasks = cls.read_tasks(data)
  File "~/localcode/anki-geo-ultimate-geography/.venv/lib/python3.7/site-packages/brain_brew/commands/run_recipe/recipe_builder.py", line 73, in read_tasks
    inner_task.execute()
  File "~/localcode/anki-geo-ultimate-geography/.venv/lib/python3.7/site-packages/brain_brew/build_tasks/crowd_anki/note_model_single_from_crowd_anki.py", line 59, in execute
    raise ReferenceError(f"Missing Note Model '{self.model_name}' in CrowdAnki file")
ReferenceError: Missing Note Model 'Ultimate Geography' in CrowdAnki file
@aplaice
Copy link
Collaborator

aplaice commented Jan 16, 2025 •
edited
Loading

Thanks for reporting!

Yes, it seems our documentation is wrong! I also don't think that it's at all possible for the same recipe to convert both the standard and extended versions of the deck to source, given the difference in note model name.

Is converting the extended deck to source something that you think useful? If so, we can easily add a recipe for that. Otherwise, we'll just fix the documentation. (I'm asking because I think that the existing (standard deck to source) recipe hasn't been used much, so I don't think we want to add a new recipe just for completeness, but only if it's at least slightly valuable.)

(I expect that a recipe to convert a deck with additional fields into our internal format would be valuable, but I don't think that a general such recipe would be currently possible with BrainBrew. Manually tweaking the recipe on a case-by-case basis is probably simplest.)

@Joilence
Copy link
Author

Is converting the extended deck to source something that you think useful?

For me anki_to_src is quite essential to version control my notes.

Was there a historical reason that extended and non-extended deck are kept? I feel like for people who don't want certain cards, it is easier to remove them in Anki app. If I didn't miss anything, anki_to_src is more about notes instead of cards, so missing expected cards should be fine?

aplaice added a commit to aplaice/anki-ultimate-geography that referenced this issue Jan 23, 2025
@aplaice
Allow converting extended deck from CrowdAnki format to source
476fce1 
Fix anki-geo#666

It would be convenient for a single recipe to exist, and (given that
we're not actually saving the note model itself to file) it should be
possible, from a theoretical flow-of-data point of view, but I don't
think this is currently supported by BrainBrew — the exact name of the
note model has to be provided.  (It would also likely be non-trivial
to do this correctly, robustly and flexibly (while also not dropping
the option of using `save_to_file` in the general case), given that
the only reason this could theoretically work is because both note
models have the exact same fields.)

Tested that the recipe works as expected by:

1. Generate the CrowdAnki files in `build/` with the source to Anki
recipe.

2. Make a change in one of the `src/data/*.csv` files. (Note: this
is necessary before *3* because otherwise BrainBrew correctly notices
there are no changes, and doesn't actually re-sort the notes, which
would make the later comparison messy.)

3. Run the Anki to source [extended] recipe.  (This resorts the notes,
but resetting the actual content to the state from before *2*.)

4. Make a change in `build/Ultimate Geography [EN]
[Extended]/deck.json`.

5. Run the Anki to source [extended] recipe.

6. Compare the state between after *3* and after *5* and verify that
your change was applied.
@aplaice aplaice linked a pull request Jan 23, 2025 that will close this issue
Open
@aplaice
Copy link
Collaborator

aplaice commented Jan 23, 2025

Was there a historical reason that extended and non-extended deck are kept? I feel like for people who don't want certain cards, it is easier to remove them in Anki app. If I didn't miss anything, anki_to_src is more about notes instead of cards, so missing expected cards should be fine?

Yes, mainly a historical reason. Yes, it might well be easier to just have the extended deck (though not 100% sure). Unfortunately, AFAIR (I haven't checked recently, so this might have changed), AnkiWeb (where the most easily accessible version of AUG is available (one which can be installed without CrowdAnki etc.)) does not enable updating the note model of existing notes, so it might be non-trivial to update existing AnkiWeb users of the deck from the standard to the extended deck. (Installing CrowdAnki and upgrading that way works but isn't very user-friendly.)

axelboc pushed a commit to aplaice/anki-ultimate-geography that referenced this issue Jan 27, 2025
@aplaice @axelboc
Allow converting extended deck from CrowdAnki format to source
f5f44f5 
Fix anki-geo#666

It would be convenient for a single recipe to exist, and (given that
we're not actually saving the note model itself to file) it should be
possible, from a theoretical flow-of-data point of view, but I don't
think this is currently supported by BrainBrew — the exact name of the
note model has to be provided.  (It would also likely be non-trivial
to do this correctly, robustly and flexibly (while also not dropping
the option of using `save_to_file` in the general case), given that
the only reason this could theoretically work is because both note
models have the exact same fields.)

Tested that the recipe works as expected by:

1. Generate the CrowdAnki files in `build/` with the source to Anki
recipe.

2. Make a change in one of the `src/data/*.csv` files. (Note: this
is necessary before *3* because otherwise BrainBrew correctly notices
there are no changes, and doesn't actually re-sort the notes, which
would make the later comparison messy.)

3. Run the Anki to source [extended] recipe.  (This resorts the notes,
but resetting the actual content to the state from before *2*.)

4. Make a change in `build/Ultimate Geography [EN]
[Extended]/deck.json`.

5. Run the Anki to source [extended] recipe.

6. Compare the state between after *3* and after *5* and verify that
your change was applied.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Allow converting extended deck from CrowdAnki format to source aplaice/anki-ultimate-geography
2 participants
@Joilence @aplaice