Extend import help#2155

Open

mlell wants to merge 7 commits into

beancount:main from

mlell:dev_import-help

Open

Extend import help #2155
mlell wants to merge 7 commits into
beancount:main from
mlell:dev_import-help

Conversation

@mlell

@mlell mlell commented Dec 13, 2025

Copy link

Copy Markdown

Contributor

I would like to suggest this expansion of the import help. It was too terse and technical to understand for me when I just discovered beancount. I figured that a GUI like fava might be a good way to learn my ways around beancount, but the import docs were too confusing for me so I ended up working around the whole import process by just generating the beancount ledgers by a self-written stand-alone script.

This is a pitty, given that fava has a nice GUI support and I find the Importer declarations of beangulp very elegant. So therefore I have extended the help page of fava in the hope that other newbie users will have an easier time to start with beancount/fava.

The import help text is less dense now and presupposes less knowledge about importing in beancount and the detailed differences of the different generations of import mechanisms (beancount.ingest vs beangulp). There are clearer step-by-step instructions and an example that should get a user floating for a basic import-CSV case. Also, the full signature of hook function is now documented, which wasn't done anywhere in the documentations of beancount, beangulp, or fava.

Additionally, this adds to the syntax overview the simple information that commoditys = currencies from the perspective of beancount. This would have helped me as a newcomer to find correct BQL functions for my needs.

There are two failing tests, but I think they are not related to this PR, as I have only edited markdown files:

FAILED tests/test_core_fava_options.py::test_fava_options_language - fava.core.fava_options.UnsupportedLanguageOptionError: Fava has no translations for: 'zh'.
FAILED tests/test_util.py::test_get_translations - assert None

yagebu

yagebu requested changes

Dec 13, 2025

View reviewed changes

@yagebu yagebu left a comment

Copy link

Copy Markdown

Member

Choose a reason for hiding this comment

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

Thanks for the PR, I've left some comments.

src/fava/help/import.md Outdated

@mlell

mlell commented Dec 16, 2025 •

edited

Loading

Copy link

Copy Markdown

Contributor Author

Thanks for taking the time to review this! I have updated the help page to include your changes and write unit tests. The example is now in tests/data/import_example_for_docs.py

The type checker has reported several errors. I have fixed those that I understand but for some I would need help...

tests/data/import_example_for_docs.py:23: error: Class cannot subclass "Importer" (has type "Any") [misc]

Offending code:

fava/tests/data/import_example_for_docs.py

Line 23 in 999f94b

class MyCSVImporter(csvbase.Importer):

I do not understand this message.

tests/data/import_example_for_docs.py:84: error: Variable "tests.data.import_example_for_docs.Row" is not valid as a type [valid-type]

I tried defining the Row type as Row: TypeAlias = "Row", however, this still gives an error:

tests/data/import_example_for_docs.py:25: error: Cannot resolve name "Row" (possible cyclic definition) [misc]

Offending code:

fava/tests/data/import_example_for_docs.py

Line 84 in 999f94b

def read(self, filepath: str) -> Row:

Row is a type which is defined on the fly by beangulp:

https://github.com/beancount/beangulp/blob/c5e8188bd186a1c77bb5835b252f2f07d41dd85b/beangulp/importers/csvbase.py#L318

I tried to put "Row" in string literals, but this is removed by the formatter. I do not know how to resolve this.

tests/data/import_example_for_docs.py:147: error: Unsupported operand type for unary - ("Amount | None") [operator]

fava/tests/data/import_example_for_docs.py

Line 147 in 999f94b

-txn.postings[0].units,

The type required by the tuple is is Optional[Amount] --- I do not know how to resolve this.

https://github.com/beancount/beancount/blob/1cbb0b6bc08a47572c4779eabee71e9fd5807079/beancount/core/data.py#L232

And finally, there is

tests/test_import_documentation_example.py:7: error: Skipping analyzing "beangulp.extract": module is installed, but missing library stubs or py.typed marker [import-untyped]

Is this because beangulp has no typing annotations? Should we exclude beangulp from mypy checking?

@mlell mlell requested a review from yagebu

December 16, 2025 12:55

@mlell mlell force-pushed the dev_import-help branch 5 times, most recently from 2d63ab1 to dbc8251 Compare

December 16, 2025 14:12

@yagebu

yagebu commented Dec 17, 2025

Copy link

Copy Markdown

Member

The latest released version of beangulp does not have a py.typed marker, so it doesn't typecheck. For the parts used in Fava, there's type stub files in the stubs directory. If it's possible to extend those without a huge effort, that's an option, otherwise feel free to add # type: ignore comments.

@mlell

mlell commented Dec 18, 2025 •

edited

Loading

Copy link

Copy Markdown

Contributor Author

I mostly included type stubs.

fava/tests/data/import_example_for_docs.py

Line 2 in cda4671

# mypy: disable-error-code="assignment"

This is to prevent these errors:

tests/data/import_example_for_docs.py:41: error: Incompatible types in assignment (expression has type "Date", base class "Importer" defined the type as "Callable[[str], date]") [assignment]
tests/data/import_example_for_docs.py:69: error: Incompatible types in assignment (expression has type "type[excel]", base class "CSVReader" defined the type as "str | Dialect") [assignment]

The first one stems from the fact that csvbase.CSVReader (superclass of csvbase.Importer) shadows the date() function of the ABC beangulp.Importer, (the other superclass). For subclasses of CSVReader, date is a column specification (csvbase.Date)

The second one makes no sense to me, because csv.excel is a subclass of csv.Dialect, as required by the type?

fava/tests/data/import_example_for_docs.py

Line 200 in cda4671

ingest = beangulp.Ingest(CONFIG, HOOKS) # type: ignore[arg-type]

This silences

tests/data/import_example_for_docs.py:200: error: Argument 2 to "Ingest" has incompatible type "list[Callable[[list[tuple[str, list[Open | Close | Commodity | Pad | Balance | <7 more items>], str, Importer]], list[Open | Close | Commodity | Pad | Balance | <7 more items>]], list[tuple[str, list[Open | Close | Commodity | Pad | Balance | <7 more items>], str, Importer]]]]"; expected "Sequence[Callable[[Sequence[tuple[str, Sequence[Directive], str, Importer]], Sequence[Directive]], Sequence[tuple[str, Sequence[Directive], str, Importer]]]]"

It expects fava.beans.abc.Directive, but gets beancount.core.data.Directive which is Union[Open, Close, ...]. I do not want to use to import fava.beans.abc in the user-facing example because the user will deal with the beancount types, not the fava ones. I don't understand why fava re-defines the Beancount types.

fava/tests/test_import_documentation_example.py

Line 7 in cda4671

import beangulp.extract # type: ignore[import-untyped]

I did not bother to make a type stub for beangulp.extract, as its only used in one test, we will notice anyway when this fails.

fava/tests/test_import_documentation_example.py

Line 43 in cda4671

ing = fava.core.IngestModule(ledger) # type: ignore[attr-defined]

This prevents

tests/test_import_documentation_example.py:43: error: Module "fava.core" does not explicitly export attribute "IngestModule" [attr-defined]

which I think is not required for a test? Unsure about what this is.

@mlell

mlell commented Dec 18, 2025 •

edited

Loading

Copy link

Copy Markdown

Contributor Author

Unfortunately, the pre-commit hook fails because the CI/CD system might have an older version of mdformat. I get

mdformat: error: unrecognized arguments: --exclude

The hook runs on my machine. I need the exclude statment, because mdformat wants to destroy the jinja2 syntax {{ ... }} in import.md which is required to create the GitHub URL for the user example.

@yagebu, can you update mdformat, or is there another way to exclude this file from mdformat treatment?

@mlell mlell force-pushed the dev_import-help branch 3 times, most recently from 66ad045 to a2aaca5 Compare

December 18, 2025 11:05

@mlell

mlell commented Dec 18, 2025 •

edited

Loading

Copy link

Copy Markdown

Contributor Author

I have found a workaround. All tests pass now and test coverage is also 100% again. There is only one configuration one which uses outdated dependencies, so the provided example does not run on outdated versions... I could live with that, its only documentation....

@mlell

mlell commented Dec 18, 2025

Copy link

Copy Markdown

Contributor Author

Skipping tests for beangulp < 2.0.0, all tests pass now

@mlell mlell force-pushed the dev_import-help branch 2 times, most recently from 4be0361 to f78d0d6 Compare

February 10, 2026 15:47

@mlell mlell force-pushed the dev_import-help branch 2 times, most recently from 99c84e7 to 495a091 Compare

March 6, 2026 20:33

@mlell


 Extend import help, add currency/commodity synonym, link to beancount...

57c69f6

... option help
This fixes a few issues that were hampering me as a newcomer to
understand the import process.
The import help text is less dense now and presupposes less knowledge
about importing in beancount and the detailed differences of the
different generations of import mechanisms (beancount.ingest vs
beangulp). There are clearer step-by-step instructions and an example
that should get a user floating for a basic import-CSV case. Also, the
full signature of hook function is now documented, which wasn't done
anywhere in the documentations of beancount, beangulp, or fava.
Additionally, this adds to the syntax overview the simple information
that commoditys = currencies from the perspective of beancount. This
would have helped me as a newcomer to find correct BQL functions for my
needs.

@mlell mlell force-pushed the dev_import-help branch 2 times, most recently from ecfeba6 to 1b2ac28 Compare

March 6, 2026 20:38

mlell added 3 commits

March 6, 2026 21:39

@mlell


 Help: Add link to unit-tested example import configuration

69d50f4

@mlell


 Help: Add link to unit-tested example import configuration

bb6c32d

@mlell


 Help: Add minimal example hook function

fe8bf46

@mlell mlell force-pushed the dev_import-help branch 2 times, most recently from 7386199 to aa0a14d Compare

March 7, 2026 10:30

mlell added 2 commits

March 7, 2026 18:41

@mlell


 Add type stubs for example import configuration

3bb0892

@mlell


 Skip testing example import configuration for old beangulp version

8a6545a

@mlell mlell force-pushed the dev_import-help branch from a5863aa to 8a6545a Compare

March 7, 2026 17:43

@pre-commit-ci


 [pre-commit.ci] auto fixes from pre-commit.com hooks

62a346d

for more information, see https://pre-commit.ci

@mlell

mlell commented Mar 7, 2026

Copy link

Copy Markdown

Contributor Author

@yagebu , Hi I just wanted to follow up on this. I have rebased the PR and cleared the tests! What do you think? :-)

@wuqs-net

wuqs-net commented May 11, 2026

Copy link

Copy Markdown

@yagebu Could you please take a look at this at your convenience? I agree it's a good idea to expand the import help.

Labels

None yet

3 participants

@mlell @yagebu @wuqs-net

Conversation

@mlell mlell commented Dec 13, 2025

Uh oh!

@yagebu yagebu left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

mlell commented Dec 16, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

yagebu commented Dec 17, 2025

Uh oh!

mlell commented Dec 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mlell commented Dec 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mlell commented Dec 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mlell commented Dec 18, 2025

Uh oh!

mlell commented Mar 7, 2026

Uh oh!

wuqs-net commented May 11, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

mlell commented Dec 16, 2025 •

edited

Loading

mlell commented Dec 18, 2025 •

edited

Loading

mlell commented Dec 18, 2025 •

edited

Loading

mlell commented Dec 18, 2025 •

edited

Loading