mm21 / trilium-alchemy

Python SDK and CLI toolkit for Trilium Notes
GNU Affero General Public License v3.0
10 stars 1 forks source link

Add Note.get(key, value) method #6

Closed jwhonce closed 1 year ago

jwhonce commented 1 year ago

Analogous to dict.get(), makes working with optional attributes simplier.

value = note["label"] if label in note else "N/A" becomes value = note.get("label", "N/A")

jwhonce commented 1 year ago

You have no contributing guidelines in the repo but habit dictates I try to help. :-) I thought this was a good, simple place to test your interest in PR's.

mm21 commented 1 year ago

Hi, thanks for submitting this! I've been meaning to add contributing guidelines, I'll prioritize that.

This is a good idea, and it overlaps with an idea I've been thinking about - Note could subclass collections.abc.MutableMapping and provide a fully-featured dict interface for attributes. Then we'd have get() for free, along with other methods like keys(), items(), etc (see the entry for MutableMapping in this table: https://docs.python.org/3/library/collections.abc.html#collections-abstract-base-classes).

Since we already have __getitem__ and __setitem__, the only missing methods to implement are:

Then we can delete __contains__ since that would be similarly provided by MutableMapping.

If you're up for making these changes that would be great, but if it's more than you bargained for I'll accept your PR and then make the revisions myself :). Ideally we'll also want tests for these methods, e.g. del note["attribute"]. I'll document the steps for developing/running tests if you're interested (this will also be in the contributing guidelines eventually).

jwhonce commented 1 year ago

I will be happy to work on sub-classing Mapping. And agree something that big needs tests. If you provide guidance, I'll write those as part of this PR

mm21 commented 1 year ago

Here's some documentation for testing: (will integrate this in the docs)

Setup

Installation

Install the dependencies by activating a shell (poetry shell) and then running poetry install --with=dev. You'll get an error about autodoc2 since it's looking for my own local fork which I haven't put on GitHub yet, but this is only for building the docs. (I'll upload my fork soon and update the dependency.)

Environment

From the root folder, copy .env.example to .env and fill in the variables. Not all tests require all of them, e.g. TRILIUM_DATA_DIR is only used to verify that backups were created. However it's encouraged to provide all of them so you can run all the tests.

Running

[!IMPORTANT] Please use a separate, "disposable" Trilium instance to run the tests. Running the full suite creates and deletes over 70 notes, and one of the tests deletes any existing notes. The runner will detect existing notes and require you to pass --clobber if you want it to proceed running tests, acknowledging existing notes may be deleted.

Run pytest followed by an optional path/test filter, e.g.:

pytest test/attribute/test_helpers.py::test_index_del

Incidentally, this test is similar to the one we need for del note["attributeName"].

Developing

Fixtures

To get a new Session and a new Note for testing, simply create a test (function starting with "test") which takes arguments called session and note respectively. For example:

def test_index_del(session: Session, note: Note):
    ...

Here a new Session will be created, along with a new Note. The note is automatically deleted after the test exits.

Markers

Markers are useful to configure certain things about the created Note under test, e.g. which attributes it has. For example, test_index_del uses such a marker to create an attribute in order to test deleting it.

@mark.attribute("label1")
def test_index_del(session: Session, note: Note):
    assert note["label1"] == ""
    ...

To test __delitem__ we can use something like above and do del note["label1"], then assert "label1" not in note.

It would be good to similarly test iterating over multiple attributes - add multiple attributes using @mark.attribute(), then make sure they're provided in the same order when iterating over note.

mm21 commented 1 year ago

Nice work! Released in trilium_alchemy-0.1.7.

Documentation for new API Note.get() is here: https://mm21.github.io/trilium-alchemy/build/autodoc2/trilium_alchemy/trilium_alchemy.core.note.html#trilium_alchemy.core.note.Note.get