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

Use Babel's format_datetime, not format_date #6981

Merged
merged 2 commits into from
Mar 31, 2024

Conversation

szg-alex-payne
Copy link
Contributor

I am using MkDocs-Material 9.5.15 and Babel 2.14.0. When setting the config value blog.post_date_format: "yyyy-MM-dd hh:mm:ss", the blog plugin crashes during compilation with the error "date objects have no hour field".

I believe this is occurring because the plugin provides a datetime to babel.dates.format_date, which upcasts it to a plain, timeless, date. Making this change to use babel.dates.format_datetime instead resolved the error and demonstrated expected behavior.

While I recognize that it is likely uncommon for a blog to want to include sub-date timestamps in any of these fields, I believe that the current documentation,

The format string must adhere to babel's pattern syntax and should not contain whitespace

implies that any of Babel's pattern markers are acceptable here. I would consider

The format string must adhere to Babel's pattern syntax for calendar dates and should not contain whitespace

to also be a sufficient fix for this case.


Thank you for your effort in creating this project; it has been a wonderful benefit to my team, and this is the first minor problem we've had in months of using it.

I am using MkDocs-Material 9.5.15 and Babel 2.14.0. When setting the config value `blog.post_date_format: "yyyy-MM-dd hh:mm:ss"`, the blog plugin crashes during compilation with the error "date objects have no hour field".

I believe this is occurring because the plugin provides a `datetime` to `babel.dates.format_date`, which upcasts it to a plain, timeless, `date`. Making this change to use `babel.dates.format_datetime` instead resolved the error and demonstrated expected behavior.

While I recognize that it is likely uncommon for a blog to want to include sub-date timestamps in any of these fields, I believe that the current documentation,

> The format string must adhere to babel's pattern syntax and should not contain whitespace

implies that _any_ of Babel's pattern markers are acceptable here. I would consider 

> The format string must adhere to Babel's pattern syntax for calendar dates and should not contain whitespace

to also be a sufficient fix for this case.

----

Thank you for your effort in creating this project; it has been a wonderful benefit to my team, and this is the first minor problem we've had in months of using it.
@squidfunk
Copy link
Owner

Thanks for the PR! It sounds legit, and should work, as we coerce all dates to datetimes anyway, but unfortunately you made the changes in the wrong place. You need to make changes in the src directory to make them permanent, as explained in our theme development guide. When done, please run npm run build:all.

Additionally, could you please provide a minimal reproduction here? Before we can consider merging a PR that fixes an error, we must first ensure that it actually is an error. This is also why, before creating an issue, we recommend to discuss changes with us. (info box) We cannot just change things without understanding their impact. However, in this case, you can attach the minimal reproduction which showcases the problem here. I hope you understand that with a project of this size, we have to have processes and we must ask our users to stick to them. Otherwise it would be unmanageable.

Copy link
Owner

@squidfunk squidfunk left a comment

Choose a reason for hiding this comment

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

See above

@szg-alex-payne
Copy link
Contributor Author

Of course! I apologize for the misplacement; I made the patch in my own app's dependency tree, and once it ran, replicated the change here.

I will submit a correct commit and a before/after diagnostic after dinner. Thank you for the clear instruction!

@szg-alex-payne
Copy link
Contributor Author

9.5.15-datetime-type-error.zip

Current behavior:

ERROR   -  Error building page 'blog/index.md': 'datetime.date' object has no attribute 'hour'
Traceback (most recent call last):
  File "<frozen runpy>", line 198, in _run_module_as_main
  File "<frozen runpy>", line 88, in _run_code
  File ".venv\Scripts\mkdocs.exe\__main__.py", line 7, in <module>
  File ".venv\Lib\site-packages\click\core.py", line 1157, in __call__
    return self.main(*args, **kwargs)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^
  File ".venv\Lib\site-packages\click\core.py", line 1078, in main
    rv = self.invoke(ctx)
         ^^^^^^^^^^^^^^^^
  File ".venv\Lib\site-packages\click\core.py", line 1688, in invoke
    return _process_result(sub_ctx.command.invoke(sub_ctx))
                           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File ".venv\Lib\site-packages\click\core.py", line 1434, in invoke
    return ctx.invoke(self.callback, **ctx.params)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File ".venv\Lib\site-packages\click\core.py", line 783, in invoke
    return __callback(*args, **kwargs)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File ".venv\Lib\site-packages\mkdocs\__main__.py", line 286, in build_command
    build.build(cfg, dirty=not clean)
  File ".venv\Lib\site-packages\mkdocs\commands\build.py", line 349, in build
    _build_page(
  File ".venv\Lib\site-packages\mkdocs\commands\build.py", line 232, in _build_page
    output = template.render(context)
             ^^^^^^^^^^^^^^^^^^^^^^^^
  File ".venv\Lib\site-packages\jinja2\environment.py", line 1301, in render
    self.environment.handle_exception()
  File ".venv\Lib\site-packages\jinja2\environment.py", line 936, in handle_exception
    raise rewrite_traceback_stack(source=source)
  File ".venv\Lib\site-packages\material\templates\blog.html", line 4, in top-level template code
    {% extends "main.html" %}
  File ".venv\Lib\site-packages\material\templates\main.html", line 4, in top-level template code
    {% extends "base.html" %}
  File ".venv\Lib\site-packages\material\templates\base.html", line 180, in top-level template code
    {% block container %}
^^^^^^^^^^^^^^^^^^^
  File ".venv\Lib\site-packages\material\templates\blog.html", line 12, in block 'container'
    {% include "partials/post.html" %}
^^^^^^^^^^^^^^^^^^^^^
  File ".venv\Lib\site-packages\material\templates\partials\post.html", line 19, in top-level template code
    {{- post.config.date.created | date -}}
^^^^^^^^^^^^^^^^^
  File ".venv\Lib\site-packages\material\plugins\blog\plugin.py", line 304, in date_filter
    return self._format_date_for_post(date, config)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File ".venv\Lib\site-packages\material\plugins\blog\plugin.py", line 791, in _format_date_for_post
    return self._format_date(date, format, config)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File ".venv\Lib\site-packages\material\plugins\blog\plugin.py", line 786, in _format_date
    return format_date(date, format = format, locale = locale)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File ".venv\Lib\site-packages\babel\dates.py", line 685, in format_date
    return pattern.apply(date, locale)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File ".venv\Lib\site-packages\babel\dates.py", line 1318, in apply
    return self % DateTimeFormat(datetime, locale, reference_date)
           ~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  File ".venv\Lib\site-packages\babel\dates.py", line 1310, in __mod__
    return self.format % other
           ~~~~~~~~~~~~^~~~~~~
  File ".venv\Lib\site-packages\babel\dates.py", line 1360, in __getitem__
    if self.value.hour % 12 == 0:
       ^^^^^^^^^^^^^^^
AttributeError: 'datetime.date' object has no attribute 'hour'

After this change, the missing-attribute error goes away, and the site builds as expected.


As a bonus, I have included a second bug in the reproduction zip: after this patch is applied, using a timezone marker in the metadata's date literal causes a comparison error:

Traceback (most recent call last):
  File "<frozen runpy>", line 198, in _run_module_as_main
  File "<frozen runpy>", line 88, in _run_code
  File ".venv\Scripts\mkdocs.exe\__main__.py", line 7, in <module>
  File ".venv\Lib\site-packages\click\core.py", line 1157, in __call__
    return self.main(*args, **kwargs)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^
  File ".venv\Lib\site-packages\click\core.py", line 1078, in main
    rv = self.invoke(ctx)
         ^^^^^^^^^^^^^^^^
  File ".venv\Lib\site-packages\click\core.py", line 1688, in invoke
    return _process_result(sub_ctx.command.invoke(sub_ctx))
                           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File ".venv\Lib\site-packages\click\core.py", line 1434, in invoke
    return ctx.invoke(self.callback, **ctx.params)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File ".venv\Lib\site-packages\click\core.py", line 783, in invoke
    return __callback(*args, **kwargs)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File ".venv\Lib\site-packages\mkdocs\__main__.py", line 286, in build_command
    build.build(cfg, dirty=not clean)
  File ".venv\Lib\site-packages\mkdocs\commands\build.py", line 304, in build
    files = config.plugins.on_files(files, config=config)
            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File ".venv\Lib\site-packages\mkdocs\plugins.py", line 533, in on_files
    return self.run_event('files', files, config=config)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File ".venv\Lib\site-packages\mkdocs\plugins.py", line 507, in run_event
    result = method(item, **kwargs)
             ^^^^^^^^^^^^^^^^^^^^^^
  File ".venv\Lib\site-packages\material\plugins\blog\plugin.py", line 133, in on_files
    self.blog.posts = sorted(
                      ^^^^^^^
  File ".venv\Lib\site-packages\material\plugins\blog\plugin.py", line 444, in _resolve_posts
    if not self._is_excluded(post):
           ^^^^^^^^^^^^^^^^^^^^^^^
  File ".venv\Lib\site-packages\material\plugins\blog\plugin.py", line 373, in _is_excluded
    return post.config.date.created > datetime.now()
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
TypeError: can't compare offset-naive and offset-aware datetimes

I'm not sure what, if anything, to do about this. I would probably order my potential solution ideas roughly like this:

  1. assume all unmarked times are in the system's local timezone, and attempt to attach that zone information when first parsing
  2. assume all unmarked times are in UTC
  3. adjust the docs, telling users not to use timezone markers

I hope I have followed your instructions sufficiently. I am not a fluent Python developer. Please let me know if there's anything else you need from me.

@szg-alex-payne
Copy link
Contributor Author

While I have run npm run build:all, I have not committed it, as I am on a Windows machine and the changes appear to be nearly all insertions of my own filesystem paths into the generated files.

@squidfunk
Copy link
Owner

I'm not sure what you're referring to, maybe the source maps? Those should be relative, at least they all are on my machine and in the repository. However, you can also only commit the contents of material/plugins and src/plugins.

@squidfunk
Copy link
Owner

Thanks for providing the reproduction – looks good to me!

@squidfunk squidfunk merged commit abfac1a into squidfunk:master Mar 31, 2024
@squidfunk squidfunk mentioned this pull request Apr 2, 2024
4 tasks
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