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

Fixes time format error when compiling under Windows #2228

Merged
merged 7 commits into from
Sep 20, 2024

Conversation

kdschlosser
Copy link
Contributor

@kdschlosser kdschlosser commented Sep 19, 2024

This should fix the error that occurs at 31% when compiling under Windows.


📚 Documentation preview 📚: https://writethedocs-www--2228.org.readthedocs.build/

Copy link
Contributor

@plaindocs plaindocs left a comment

Choose a reason for hiding this comment

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

LGTM

@plaindocs plaindocs enabled auto-merge (squash) September 19, 2024 16:08
@vwheeler63
Copy link
Collaborator

@kdschlosser I am still getting an error at 31% through the read step. Here is the exception trace:

Traceback (most recent call last):
  File "e:\dev\clients\wga\writethedocs\www\venv\lib\site-packages\sphinx\events.py", line 94, in emit
    results.append(listener.handler(self.app, *args))
  File "E:\Dev\Clients\WGA\writethedocs\www\docs\_ext\core.py", line 221, in render_rst_with_jinja
    conf_context = load_conference_page_context(app, docname)
  File "E:\Dev\Clients\WGA\writethedocs\www\docs\_ext\core.py", line 61, in load_conference_page_context
    context = load_conference_context_from_yaml(shortcode, year, year_str, page)
  File "E:\Dev\Clients\WGA\writethedocs\www\docs\_ext\core.py", line 144, in load_conference_context_from_yaml
    schedule_item['time'] = naive_item_start.strftime(TIME_FORMATS[data['time_format']])
ValueError: Invalid format string

The above exception was the direct cause of the following exception:

Traceback (most recent call last):
  File "e:\dev\clients\wga\writethedocs\www\venv\lib\site-packages\sphinx\cmd\build.py", line 276, in build_main
    app.build(args.force_all, filenames)
  File "e:\dev\clients\wga\writethedocs\www\venv\lib\site-packages\sphinx\application.py", line 330, in build
    self.builder.build_update()
  File "e:\dev\clients\wga\writethedocs\www\venv\lib\site-packages\sphinx\builders\__init__.py", line 286, in build_update
    self.build(to_build,
  File "e:\dev\clients\wga\writethedocs\www\venv\lib\site-packages\sphinx\builders\__init__.py", line 300, in build
    updated_docnames = set(self.read())
  File "e:\dev\clients\wga\writethedocs\www\venv\lib\site-packages\sphinx\builders\__init__.py", line 407, in read
    self._read_serial(docnames)
  File "e:\dev\clients\wga\writethedocs\www\venv\lib\site-packages\sphinx\builders\__init__.py", line 428, in _read_serial
    self.read_doc(docname)
  File "e:\dev\clients\wga\writethedocs\www\venv\lib\site-packages\sphinx\builders\__init__.py", line 468, in read_doc
    doctree = read_doc(self.app, self.env, self.env.doc2path(docname))
  File "e:\dev\clients\wga\writethedocs\www\venv\lib\site-packages\sphinx\io.py", line 181, in read_doc
    pub.publish()
  File "e:\dev\clients\wga\writethedocs\www\venv\lib\site-packages\docutils\core.py", line 217, in publish
    self.document = self.reader.read(self.source, self.parser,
  File "e:\dev\clients\wga\writethedocs\www\venv\lib\site-packages\sphinx\io.py", line 100, in read
    self.input = self.read_source(settings.env)
  File "e:\dev\clients\wga\writethedocs\www\venv\lib\site-packages\sphinx\io.py", line 110, in read_source
    env.events.emit('source-read', env.docname, arg)
  File "e:\dev\clients\wga\writethedocs\www\venv\lib\site-packages\sphinx\events.py", line 102, in emit
    raise ExtensionError(__("Handler %r for event %r threw an exception") %
sphinx.errors.ExtensionError: Handler <function render_rst_with_jinja at 0x000002CC288A7280> for event 'source-read' threw an exception (exception: Invalid format string)

Extension error (_ext.core):
Handler <function render_rst_with_jinja at 0x000002CC288A7280> for event 'source-read' threw an exception (exception: Invalid format string)

@kdschlosser
Copy link
Contributor Author

Time formatting in Python is actually a really sore point for me because the format identifiers are not the same across the different OS's that Python runs on. Why it is like this is a big ?. I don't see how the OS should play any role in what time format specifiers work and which ones don't.

Give me a few minutes to hammer out which ones are going to play nice with Windows. I want the output to appear the same but that might not be possible using a simple format specifier. I will have to go about it in a different way if that is the case.,

@plaindocs
Copy link
Contributor

yeah, that was pretty wild to learn!

@kdschlosser
Copy link
Contributor Author

OH no!! I can't push a commit to the PR.. something happened and it's not letting me. did you make a change to the PR?

@kdschlosser
Copy link
Contributor Author

I got it sorted..

This will hopefully work now...

I wanted to keep the leading zero for the hour stripped off. that is what the %-I specifier does. but this is not available under Windows., don't ask me why it's not it just isn't. The %I is available but the leading zero is there. so I just wrote a function to handle the removal of the leading zero.

@vwheeler63
Copy link
Collaborator

vwheeler63 commented Sep 19, 2024

I don't see how the OS should play any role

I concur! Are you able to do the build on your system? There are instructions here in the README how to set it up. The virtual environment makes a lot of sense since it calls for Python3.8 whereas I suspect you are normally using 3.12 as I am.... With the exception trace, does that make it possible to set a stop-point under PyCharm and look at the values being passed?

@vwheeler63
Copy link
Collaborator

vwheeler63 commented Sep 19, 2024

@kdschlosser @plaindocs

Woo hoo!! 🎉 We have lift off! It finished, and generated 149.2 MB of files! Bravo, Kevin!

image

=-=-=-=-=-=

The last thing APPEARS to be a warning which I haven't figured out yet. This warning is emitted after the READ phase:

E:\Dev\Clients\WGA\writethedocs\www\docs\about\learning-resources.rst:5: WARNING:
   toctree contains reference to nonexisting document 'videos/index'

What I don't understand about it is that ./docs/about/learning-resources.rst has a .. toctree:: directive that references

/videos/index

and according to the Sphinx documentation, toctree file references that begin with a slash are SUPPOSED to be relative to the directory from which the Sphinx build is running. I am CD-ing into ./docs/ before running make html (default directory being the docs folder). And on my computer the

./docs/videos/index.rst

file is present.

image

I thought for a minute that perhaps there might have been a conflict with the Videos section heading, but changing it doesn't resolve anything, and the syntax APPEARS to be correct -- and the same syntax is working for ./docs/books/index.rst. I also tried changing /videos/index to a relative path ../videos/index but it generates the same warning. So I am stumped....

And it's NOT happening on the Linux build.... Evidence: the "videos" link works in the on-line version of the website, whereas in the learning-resources.html file generated on my system, it suffers from a missing file.

Kevin, do you recognize that at all? Are you able to reproduce it on your system?

@kdschlosser
Copy link
Contributor Author

I am pretty sure I know what is causing it.

@kdschlosser
Copy link
Contributor Author

you need to set an environment variable

set BUILD_VIDEOS=true

@plaindocs
Copy link
Contributor

Nice work folks!

Yep, on the money with the videos, some sketchy docs about it, we turned them off for local builds because it takes forever.

@plaindocs plaindocs enabled auto-merge (squash) September 20, 2024 08:35
@plaindocs
Copy link
Contributor

Merging this, and then updating the Windows CI branch. Exciting times.

@plaindocs plaindocs merged commit c037f99 into writethedocs:main Sep 20, 2024
6 checks passed
@vwheeler63
Copy link
Collaborator

you need to set an environment variable

set BUILD_VIDEOS=true

cc: @plaindocs

**laughing at self** Silly me. I see in the README it does indeed say that, and it also acknowledges it as "needs documenting and fixing".

Thanks to both of you! Well done, Kevin! 😃

Kind regards,
Vic

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.

3 participants