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.

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

Using mkdocs-table-reader-plugin as a module/pluglet for Mkdocs-Macros? #57

Closed
fralau opened this issue Feb 24, 2024 · 10 comments
Closed

Comments

@fralau
Copy link

fralau commented Feb 24, 2024

@timvink I am the developer of Mkdocs-Macros. I am contacting you since I noticed that you have given thought to the production of reproducible reports.

I noticed that several MkDocs plugins operate with the idea of using a "macro" style notation inside a Markdown page, e.g.:

{{ read_csv('cities.csv')}}

So my idea was to put together a generalized plugin that would provide a powerful reference syntax (Jinja2), as well as all the boilerplate (template engine, error management, etc., while leaving some flexibility). The developer would only have to focus on developing the Python functions (which would then become macros within the Markdown page). It works and the plugin is used by many Mkdocs projects (more than 2'500, according to Github).

To facilitate that migration effort, I implemented the concept of pluglets, which are installable as any Python package. Basically, any plugin that uses the concept of macros (like this table reader) could be rewritten by their author. But of course, that would be a matter of time, effort versus return on investment, etc.

However, there is a "transient" issue because Mkdocs-Macros and the other existing plugins that use a similar syntax might interfere with each other (depending on in which order they are declared and so on). This is a question that comes up a lot.

So, in the mean time, what if we used the code of an existing plugin in Mkdocs Macros?

To get an idea of how this idea could work in practice, I tried the experiment of writing a module that would import the applicable code of mkdocs-table-reader-plugin. It was very straightforward.

Here is the minimal code for a module (main.py in the root directory of the MkDocs project):

from mkdocs_table_reader_plugin.readers import *

def define_env(env):
    """
    This is the hook for the functions (new form)
    """
    for reader in READERS.values():
        env.macro(reader)

It does the following: it lists all the readers, which are conveniently listed in READERS (['read_csv', 'read_table', 'read_fwf', 'read_excel', 'read_yaml', 'read_json', 'read_feather', 'read_raw']) and declares them as macros. And voilà they were imported as macros available in every markdown page! Now its even possible to add programming logic (for repetitive actions), etc..

Screenshot Page

I attached the whole Mkdocs test project (mkdocs-tables-module.zip), so that you could reproduce my experiment, if you wish. It only requires a pip-install mkdocs-macros-plugin.

I just wished to know what you think about it, and if there could be potentially workable ideas there?

@timvink
Copy link
Owner

timvink commented Feb 27, 2024

Great package, mkdocs-macros ! Thanks for all the love put into that one :)

Your example is actually really clean! I used regex and some obscure logic to safely parse the args and kwargs from the matched group. I benchmarked it, and the overhead is very small. I do recognize the problem of interfering jinja2 environments, hence this ugly bit to raise a warning when macros is loaded in the wrong order:

for post_load_plugin in ["macros", "markdownextradata"]:
if post_load_plugin in plugins:
if plugins.index("table-reader") > plugins.index(post_load_plugin):
raise ConfigurationError(f"[table-reader]: Incompatible plugin order: Define 'table-reader' before '{post_load_plugin}' in your mkdocs.yml.")

if there could be potentially workable ideas there?

I have seen your concept of pluglets. To be honest, I don't like depending on mkdocs-macros for several reasons:

  • Additional mental overhead to just load a table; you would need to learn another package
  • This package has heavy dependency you don't always want; so easy to turn off
  • I have a lot of custom options around filepaths, which is useful when you have a large amount of tables
  • I am planning to support additional syntax options, to facilitate users that use obsidian (see Add superfence notation #45)

Here's another idea. To improve compatible with mkdocs-macros we might be able to register the macros that are handled by this plugin. In mkdocs 1.4+ :

The presence of an on_startup method (even if empty) migrates the plugin to the new system where the plugin object is kept across builds within one mkdocs serve (https://www.mkdocs.org/dev-guide/plugins/#on_startup)

We might be able to use on_startup() to have this plugin and macros talk to each other. Now what we might be able to do is register the table-reader 'macros' within mkdocs-macros, so that it will ignore those when validating the environment. Then users wouldn't have to bother with import order anymore. And the same mechanism could be used for other plugins that use the jinja2 syntax, like markdownextradata or https://github.com/timvink/mkdocs-git-revision-date-localized-plugin

@fralau
Copy link
Author

fralau commented Feb 28, 2024

Context

Your points about not wishing to rely on Mkdocs-Macros are well received (they make sense, and I defend freedom of choice for the members of the ecosystem).

Having developed a custom parser is actually an advantage for your plugin, since it is not perturbed by the presence of other similar syntax (hence it can be run before Mkdocs-Macros). I understand a solution based on that kind of tolerance would be self-evident.

Issue

Unfortunately, it's easier said than done for Jinja2, bacause (at least to my knowledge) it structurally cannot do that. Any two plugins that rely on Jinja2 are plainly incompatible among themselves, because whichever runs first will be thrown off by the macros declared in the other.

Once code operates within Jinja2, it is no longer dealing with the full calling string, but with the function read_csv(); its body could return nothing, something or even its own name. But there is no mechanism to write code that will return the whole outer string {{ read_csv('cities.csv')}} (the ability for a function to extract that context information at runtime would be called reflection). We could imagine "something" that would sort of reconstruct it, but my intuition tells me that going through contorsions to reinvent a string that already exists, would be a rabbit hole.

Solution

The only solution I see for the moment, would be pre-processing, by some other piece of software, which would replace the strings to be ignored ({{ read_csv('cities.csv')}}) by placeholders and then put them back (it's a standard approach to that type of problem). Actually, this question is so general, that it would interest the Jinja2 community at large (as a generalized case of "how to make two Jinja2 templating engines operate on the same file"?). I will open a discussion or issue on Jinja2.

Other points

Regardless of that issue, your idea of "registering" macros with Mkdocs-Macros, would mean to agree on a simple data structure (let's assume a plain list, to start with) which we could call, e.g. registered_macros. If my understanding is correct, you would expact that whenever Mkdocs-Macros finds an item in that list, it should leave the whole string untouched?

E.g.: {{ read_csv('cities.csv')}} would be interpreted, the system would say "wait, this is not for me", and the whole calling string should be preserved.)

The idea of relying on on_startup() is intriguing. But is it truly necessary?

@fralau
Copy link
Author

fralau commented Feb 28, 2024

I opened a discussion on Jinja2's repository: pallets/jinja#1943

@fralau
Copy link
Author

fralau commented Feb 28, 2024

Note that if you need a quick fix solution to guarantee that MkDocs-Macros won't break the call any of your plugin's functions, it would be sufficient to write:

{% raw %}
{{ read_csv('cities.csv')}}
{% endraw %}

I realize it's a little verbose, but it could also envelop a section of a page, or even a full page.

@timvink
Copy link
Owner

timvink commented Mar 8, 2024

Thanks for explaining so clearly, I understand the issue with two jinja2 templating engines. Indeed the proposal for a 'tolerant mode' (that you bring up in pallets/jinja#1943) could work.. it would leave missing macros untouched. The downside of course is that now you're never sure if your input macros are valid; which is what I have in this plugin. for example {{ read_csvs() }} (note the extra s ) is a typo and should ideally raise a 'reader not found' error.

Coming back to the idea of using on_startup and having plugins talk to each other. We could do this:

  • If mkdocs-macros sees table-reader in the list of plugins (and it's enabled), then it could go ahead and register the READERS like you showed in this issue
  • If table-reader seems mkdocs-macros in the list of plugin, it should disable itself, as the mkdocs-macros plugin will take care of itself.

It's nice, because now you always have interop, users get notified of typos, and mkdocs-macros does not need to rely on the (quite heavy) pandas dependency that table-reader has.

We might even be able to generalize this further, as I have more plugins that use regex to replace jinja2 plugins. If we agree on exposing a JINJA_MACROS dictionary (instead of the READERS we have here), then mkdocs-macros could detect if a plugin has it, register those. And the plugin could disable the regex replacement if it detect mkdocs-macros is there.

Thoughts?

@fralau
Copy link
Author

fralau commented Mar 9, 2024

@timvink It's a brilliant course of thought!

A question to solve, would be the human coordination.

I believ it would be more practical if the original writer of the plugin wrote their own pluglet, since it's simple to do, and they know how it works and how that want the pluglet to behave (especially in the corner cases, etc.). Plus there is only one of me 🙂 .

For the sake of simplicity (of development and maintenance), don't you think it would be better (rather to manage a coexistence) to simply cause a graceful failure of MkDoc's serve or build process, with a recommendation to install the corresponding pluglet?

The pluglet's installation would consists of installing the pluglet (with pip) and declaring it in the config file (mkdocs.yml ).

MkDocs-Macros can implement that graceful failure; but I would prefer not to create a precedent where MkDocs-Macros is one-sidedly discriminating against other plugins.

Perhaps we could do it in a coordinated way 🤔 : MkDocs-Macros could register, in an equivalence file, the plugins and equivalent replacement pluglets? In that case, I could fairly easily implement the change (once) in MkDocs-Macros. And then it would be MkDocs-Macros that fails in case of a collision with an incompatible plugin, with the recommendation for the user to update their own config file with the pluglet. You wouldn't have to change your plugin's code; only to provide the pluglet (a short piece of code) and let me know so that I can update the equivalence file.

What do you think?

@fralau
Copy link
Author

fralau commented Aug 1, 2024

I was looking back at this issue. Is there still some interest in it?

@timvink
Copy link
Owner

timvink commented Aug 15, 2024

I had to revisit this as I wanted to combine macros with table-reader to dynamically insert a list of tables.

I managed to get it working, see #68. The core solution was this:

if "macros" in config.plugins:
config.plugins['macros'].macros.update(self.readers)
config.plugins['macros'].variables['macros'].update(self.readers)
config.plugins['macros'].env.globals.update(self.readers)
self.external_jinja_engine = True
else:
self.external_jinja_engine = False

Now.. if you have macros enabled, it will use the jinja2 engine from macros to process all the reader tags. If it doesn't, it will use regex to replace the {{ read_* }} tags.

A second tricky part I solved was related to the search paths. A jinja2 macro is of course not aware of the source file page it is currently run in, or the docs_dir or config_dir for that matter. Solved that using an additional mkdocs event combined with a decorator that has the plugin config in it's state.

The good news: we now have backwards-compatible support for the macros plugin, and I can now do cool things like:

# index.md

{% set table_names = ["basic_table.csv","basic_table2.csv"] %}
{% for table_name in table_names %}

{ { read_csv(table_name) }}

{% endfor %}

Will release to pypi soon

@timvink
Copy link
Owner

timvink commented Aug 15, 2024

@timvink timvink closed this as completed Aug 15, 2024
@fralau
Copy link
Author

fralau commented Aug 26, 2024

See also: squidfunk/mkdocs-material#5933

@timvink Interestingly, it should auto-document if you type {{ macros_info() }} in a page.

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

No branches or pull requests

2 participants