-
Notifications
You must be signed in to change notification settings - Fork 95
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
Simply import "version" information from "traitsui.__init__" #1732
Conversation
Previously, we manually traversed the file system to extract the version and release information from traitsui._version file. Now, we simply expect traitsui to be installed in the python environment when we try building the docs and we just import version from traitsui.__init__. Note that this commit also removes the release variable in the configuration file modified: docs/source/conf.py
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The changes LGTM given your PR description. I am unsure how to test if the readthedocs build gets fixed though.
I built the docs locally and the version is pulled in correctly. eg I see TraitsUI 7.3.0.dev555 User Manual
. However, previously it looks like we had intentionally cut off the dev stuff with .split('.', 2)[:2])
. When we actually publish the docs though using a released version this won't matter.
Also is this something we will want to do in other ets projects? I notice many of them still follow an approach similar to the old way done here
version_path = os.path.join(base_path, '..', '..', 'traitsui', '_version.py') | ||
with open(version_path, 'r', encoding='utf8') as fp: | ||
exec(compile(fp.read(), version_path, 'exec'), d) | ||
release = d['version'] | ||
version = '.'.join(d['version'].split('.', 2)[:2]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Other ets packages still seem to be following this approach.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah. I need to fix that. I am not sure why we were following this approach - i'll ask first on the internal slack channel and then gut it from ETS projects.
I was planning on triggering a new RtD build after merging this PR - and i'm going off the fact that these changes exist in traits-futures where there are no RtD docs build errors.
For read the docs, i think it's actually a good idea to not remove the dev stuff from the version information because RtD automatically builds from the latest commit on the default branch. It would be confusing for RtD docs users if the "dev" didn't show up. |
this PR did in fact fix the RtD build - https://readthedocs.org/projects/traitsui/builds/14278933/ |
Note that the API documentation is broken because RtD doesn't have the custom build step where api docs are generated. We need to take the approach that traits-futures uses to have a single sphinx build step that builds both api docs and normal docs - enthought/traits-futures#348 |
Previously, we manually traversed the file system to extract the
version
andrelease
information fromtraitsui._version
file. Now, we simply expecttraitsui
to be installed in the python environment when we try building the docs and we just importversion
fromtraitsui.__init__
. Note that this PR also removes therelease
variable in the configuration file.Note that these changes should fix the broken RtD docs build. Ref https://readthedocs.org/projects/traitsui/builds/14256791/
Checklist
Add a news fragment if this PR is news-worthy for end users. (see docs/releases/README.rst)