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

Fix import PR docstrings #72

Merged
merged 3 commits into from
May 19, 2021
Merged

Conversation

CPBridge
Copy link
Collaborator

Fixing docstrings and sphinx pages for #70 and setting up this PR to merge into #70. See comments in #70 for why this is necessary. Steps involve:

  • Removing all packages whose contents moved "up the tree" from the package.rst so their docs are not duplicated.
  • Changing all references to these types in the docstrings to use the "higher level" version (with copious sed -i) so that hyperlinks in the docs build correctly.

I also fixed an issue in which highdicom.seg.content was not included in highdicom.seg.__init__.py

@CPBridge CPBridge requested a review from hackermd May 19, 2021 04:28
@CPBridge CPBridge mentioned this pull request May 19, 2021
@CPBridge
Copy link
Collaborator Author

One further thought - if and when these changes are merged into master, readthedocs will get an update with this new version of the docs with all the new shorter import paths, which will no longer match the latest release on pypi and could cause quite a lot of confusion. We want to consider making a release as soon as this is merged or holding back the readthedocs hook for a while.

Copy link
Collaborator

@hackermd hackermd left a comment

Choose a reason for hiding this comment

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

Thanks @CPBridge for going through all of the files and updating the docs!!

@hackermd hackermd merged commit 448351f into feature/imports May 19, 2021
@hackermd hackermd deleted the bug/fix_import_docstrings branch May 19, 2021 12:39
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