Skip to content

Conversation

@mlell
Copy link
Contributor

@mlell mlell commented Dec 13, 2025

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

… 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.
Copy link
Member

@yagebu yagebu left a comment

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.

@mlell
Copy link
Contributor Author

mlell commented Dec 16, 2025

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:

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:

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]

-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
@yagebu
Copy link
Member

yagebu commented Dec 17, 2025

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
Copy link
Contributor Author

mlell commented Dec 18, 2025

I mostly included type stubs.

# 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?

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.

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.

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
Copy link
Contributor Author

mlell commented Dec 18, 2025

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 2 times, most recently from 5cdbed4 to 66ad045 Compare December 18, 2025 11:04
@mlell
Copy link
Contributor Author

mlell commented Dec 18, 2025

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
Copy link
Contributor Author

mlell commented Dec 18, 2025

Skipping tests for beangulp < 2.0.0, all tests pass now

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