HGVSnomenclature / hgvs-nomenclature

HGVS Nomenclature website
https://hgvs-nomenclature.org/
MIT License
5 stars 6 forks source link

updated mkdoc plugin order; fixed style bug #187

Closed reece closed 1 month ago

ifokkema commented 1 month ago

This change breaks mkdocs on my local installation:

$ mkdocs serve
INFO    -  Building documentation...
INFO    -  [macros] - Macros arguments: {'module_name': 'main', 'modules': [], 'render_by_default': True, 'include_dir': '', 'include_yaml': [], 'j2_block_start_string': '', 'j2_block_end_string': '', 'j2_variable_start_string': '', 'j2_variable_end_string': '',
           'on_undefined': 'keep', 'on_error_fail': False, 'verbose': False}
INFO    -  [macros] - Extra variables (config file): ['analytics', 'social']
INFO    -  [macros] - Extra filters (module): ['pretty']
Error: [table-reader]: Incompatible plugin order: Define 'table-reader' before 'macros' in your mkdocs.yml.

I'm in my virtual environment and tried the commands from the README again just to be sure (pip install, uv pip install, etc). I don't know how this is not an issue on RTD. @reece, does mkdocs still work for you locally with this new code?

reece commented 1 month ago

@ifokkema Yes, it works locally for me, and did not work before yesterday's fix.

The facts on the table make me wonder whether the plugin order error depends on a version of mkdocs or one of the plugins.

@ifokkema, let's collect some data about what works/doesn't work.

snafu (linux, Python 3.12)

Installation exactly as described in the README with new venv.

❯ uv pip freeze | rg 'mkdocs|mark|pymdown'
Using Python 3.12.3 environment at venv
markdown==3.7
markdown-captions==2.1.2
markdown-exec==1.9.3
markupsafe==3.0.1
mkdocs==1.6.1
mkdocs-autolinks-plugin==0.7.1
mkdocs-awesome-pages-plugin==2.9.3
mkdocs-exclude==1.0.2
mkdocs-get-deps==0.2.0
mkdocs-glightbox==0.4.0
mkdocs-macros-plugin==1.3.4
mkdocs-material==9.5.39
mkdocs-material-extensions==1.3.1
mkdocs-mermaid2-plugin==1.1.1
mkdocs-multirepo-plugin==0.8.3
mkdocs-simple-hooks==0.1.5
mkdocs-table-reader-plugin==3.1.0
pymdown-extensions==10.11.2
ifokkema commented 1 month ago

@ifokkema, let's collect some data about what works/doesn't work.

Good idea;

$ uv pip freeze | grep -E 'mkdocs|mark|pymdown'
Using Python 3.10.12 environment at /www/git/hgvs-nomenclature/venv
markdown==3.4.4
markdown-captions==2.1.2
markdown-exec==1.7.0
markupsafe==2.1.3
mkdocs==1.5.3
mkdocs-autolinks-plugin==0.7.1
mkdocs-awesome-pages-plugin==2.9.2
mkdocs-exclude==1.0.2
mkdocs-glightbox==0.3.4
mkdocs-macros-plugin==1.0.4
mkdocs-material==9.4.3
mkdocs-material-extensions==1.2
mkdocs-mermaid2-plugin==1.1.1
mkdocs-multirepo-plugin==0.6.3
mkdocs-simple-hooks==0.1.5
mkdocs-table-reader-plugin==2.0.1
pymdown-extensions==10.3
(venv)

There are quite a few differences. I'll try upgrading packages one by one and see if I can pinpoint it.

ifokkema commented 1 month ago

Updating mkdocs-table-reader-plugin to version 3.1.0 solved the problem. I'll build a new venv and see what my versions are then; if I can't run mkdocs with the defaults, I'll set a version for the table reader plugin in requirements.txt.

ifokkema commented 1 month ago

@reece When rebuilding my venv, all my package versions are the same as yours. Only my Python version differs. Since rebuilding the venv solves the problem, new users won't have this issue. Only users who built their venv some time ago and now pull in main will have issues. Do you think we should still update the requirements.txt?

reece commented 1 month ago

I recommend leaving requirements.txt as is.

We could consider pinning, but I don't think it's worth the effort, and often creates more problems than it's worth.

On Wed, Oct 9, 2024, 12:08 Ivo Fokkema @.***> wrote:

@reece https://github.com/reece When rebuilding my venv, all my package versions are the same as yours. Only my Python version differs. Since rebuilding the venv solves the problem, new users won't have this issue. Only users who built their venv some time ago and now pull in main will have issues. Do you think we should still update the requirements.txt?

— Reply to this email directly, view it on GitHub https://github.com/HGVSnomenclature/hgvs-nomenclature/pull/187#issuecomment-2403195786, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAA2XDNAFRG26QO2WQIVUTDZ2V5KDAVCNFSM6AAAAABPTFNFZCVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDIMBTGE4TKNZYGY . You are receiving this because you were mentioned.Message ID: @.***>

ifokkema commented 1 month ago

I recommend leaving requirements.txt as is. We could consider pinning, but I don't think it's worth the effort, and often creates more problems than it's worth.

Purely for my own understanding, since I'm not very familiar with this, I understand how mkdocs-table-reader-plugin==3.1.0 would cause issues, but what about mkdocs-table-reader-plugin>=3.1.0? I can't think of potential issues, but maybe I'm overlooking something?

reece commented 1 month ago

Nope that's a good solution.

If we're going to pin versions, I would use the tilde syntax like this:

mkdocs-table-reader-plugin~=3.1

For packages that support semantic versioning, I find that this is a good trade-off between staying up-to-date and not getting surprised by changes.

ahwagner commented 1 month ago

Just FYI; ran into this exact same issue today and was grateful for the discussion here. I didn't rebuild venv, just updated this package to 3.1.0.