Skip to content

Make _load_inventory accept lists as arguments#511

Merged
pawamoy merged 1 commit intomkdocstrings:masterfrom
ssbarnea:feat/domains
Jan 19, 2023
Merged

Make _load_inventory accept lists as arguments#511
pawamoy merged 1 commit intomkdocstrings:masterfrom
ssbarnea:feat/domains

Conversation

@ssbarnea
Copy link
Copy Markdown
Contributor

@ssbarnea ssbarnea commented Jan 17, 2023

This change should enable us to pass any kind of configuration arguments to the inventory
and unblock mkdocstrings/python#49 which was not able to load
sequences as PYYAML loads them as lists, which are mutable and not allowed by pycache.

As this method is not expected to modify any of the received data, there is no problem to
convert its non-mutable list arguments into tuples using a small wrapper.

Partial: #510

Copy link
Copy Markdown
Member

@pawamoy pawamoy left a comment

Choose a reason for hiding this comment

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

Ha, didn't think of decorating the function! I like it, thanks 🙂
Quality checks will probably fail though, variables must have names longer than 1 character, also it will complain about missing parameters in the docstring, so you can just remove the docstring and make the decorator private.

Maybe just add a comment above the decorators (or next to @list_to_tuple if it's short enough) to explain why we need to convert lists to tuples, so we don't forget.

ssbarnea added a commit to ssbarnea/mkdocstrings that referenced this pull request Jan 18, 2023
I am making this contribution just to avoid depending on a approval
on every push I make on mkdocstrings#511

Once it gets it, GHA should no longer ask for approval. Still, I do
find these fixed as a minor improvement anyway.
@pawamoy
Copy link
Copy Markdown
Member

pawamoy commented Jan 19, 2023

Thanks a lot!

@pawamoy pawamoy merged commit 105ed82 into mkdocstrings:master Jan 19, 2023
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