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

Rework documentation generation #869

Merged
merged 105 commits into from
Nov 15, 2024
Merged

Rework documentation generation #869

merged 105 commits into from
Nov 15, 2024

Conversation

bpkroth
Copy link
Contributor

@bpkroth bpkroth commented Oct 9, 2024

Pull Request

Title

Documentation generation rework.


Description

This PR does an initial revamp of the way we generate and write documentation.

The goal is to simplify the generation process a bit by removing some hacks but also enable more strict checking of the documentation produced so that links resolve properly so that it is easier to navigate.

This PR is mostly the mechanics and the associated fixups to do that.
Future PRs can handle additional documentation in docstrings.

Below is a more detailed description of some of the changes.

  • Apply black and pylint to the sphinx conf.py
  • Treat sphinx warnings as errors
  • Reorg doc generation using autoapi to handle cross referencing warnings and drop use of sphinx-apidoc
  • Update overview.rst to follow suite
    • Alternatively, move overview text into the docstrings and "just" use autoapi for everything.
      • Use :py:func:, :py:class:, :py:meth:, :py:mod:, etc. to cross reference functions and classes in the docstrings.
      • Add napolean to parse numpydoc style docstrings
        (previous numpydoc method was incomplete)
      • Use intersphinx and nitpick to check (or ignore where not possible) all internal and most external cross references to ensure good code navigation
        • Fixup all broken references by either adding fully qualified types in docstrings or in the imports.
        • Ignore type aliase for for now
          (documented bug, will address in a future PR)
      • Add a few high level examples in docstrings.
        • May need from_config_string style loaders to help with this.
      • Add doctest to pytest to validate examples.
  • Update mlos_bench.run help usage output to an rst file and link that into the man table of contents tree in the output.

Type of Change

  • 🛠️ Bug fix
  • 📝 Documentation update
  • 🧪 Tests

Testing

  • Adds additional checks and linting.

  • Must pass CI

  • Manual testing as follows:

    # SKIP_COVERAGE tweak is optional - just avoids a `pytest` job dependency
    make SKIP_COVERAGE=true doc
    ./doc/nginx-docker.sh restart

    Browse to http://localhost:8080 and check the results.


Additional Notes

Future PRs can add additional documentation strings for the mlos_bench classes including examples.

In particular

  • mlos_bench.optimizers
  • mlos_bench.services

@bpkroth bpkroth added WIP Work in progress - do not merge yet bug Something isn't working documentation Improvements or additions to documentation ci labels Oct 9, 2024
@bpkroth bpkroth mentioned this pull request Oct 14, 2024
bpkroth added a commit that referenced this pull request Oct 14, 2024
# Pull Request

## Title

Quick fixups to the documentation build.

---

## Description

- Expands one of the ignored warnings.
- Reformats the sphinx conf for black and pylint.
- Removes a warning from that file around redefinition of
`html_theme_path`.
- Adds a dependency requirement tweak for `pyparsing` when installing
under python 3.8 (unrelated other CI bug).
- Adds a workaround to a mypy false positive with in checking `np.e` as
a "deleted variable" (e.g., `e` is used in a try/except block)

---

## Type of Change

- 🛠️ Bug fix
- 📝 Documentation update

---

## Testing

- Ran `make doc` and `make check-doc`.
- Ran `make notebook-exec-test`.

---

## Additional Notes (optional)

This is a quick fix to get the CI pipeline working again.
The more complete fix to remove warning ignores from doc generation,
improve cross reference linking, etc. will be handled later in #869

---
@bpkroth
Copy link
Contributor Author

bpkroth commented Oct 28, 2024

@motus this is the wip PR I was mentioning. I think moving more of our docs directly into docstrings and making use of sphinx-autoapi should help make the Github Pages version of the documentation a little bit easier to consume without having to switch back and forth with the source code markdown quite so often.

I'd like for us to add more and more examples and explanations of what and how to think about the different configs, environments, etc. there with references back to the markdown files in the source code as necessary.

Separately, we should probably add Discussions or Wiki pages or something here to allow more people to ask and answer questions about how to do things and get started.

@bpkroth
Copy link
Contributor Author

bpkroth commented Nov 7, 2024

Copy link
Member

@motus motus left a comment

Choose a reason for hiding this comment

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

Did the first pass, left some nitpicks. My biggest question so far is the mismatch between the imports and the way we reference to them in docstrings. Please see my comments re: datetime.datetime, pandas vs. pd, ConfigSpace.ConfigurationSpace, tempfile and such

doc/source/conf.py Outdated Show resolved Hide resolved
doc/source/conf.py Show resolved Hide resolved
doc/source/conf.py Outdated Show resolved Hide resolved
doc/source/conf.py Outdated Show resolved Hide resolved
mlos_core/mlos_core/optimizers/optimizer.py Show resolved Hide resolved
@bpkroth bpkroth marked this pull request as ready for review November 15, 2024 00:02
@bpkroth bpkroth requested a review from a team as a code owner November 15, 2024 00:02
@bpkroth bpkroth enabled auto-merge (squash) November 15, 2024 00:03
@bpkroth bpkroth added ready for review Ready for review and removed WIP Work in progress - do not merge yet labels Nov 15, 2024
@bpkroth
Copy link
Contributor Author

bpkroth commented Nov 15, 2024

Did the first pass, left some nitpicks. My biggest question so far is the mismatch between the imports and the way we reference to them in docstrings. Please see my comments re: datetime.datetime, pandas vs. pd, ConfigSpace.ConfigurationSpace, tempfile and such

Yeah, I fought with that a bit, but gave up in the interest of getting something working.

For some reason it appears to correctly resolve from the imports most of the things, but not all of them.

Rather than fight with it or disable the nitpick checking (since I think that's valuable for catching broken reference links to make sure our doc navigation works), I just changed the ones that complained to use fully qualified names.

@bpkroth bpkroth requested a review from motus November 15, 2024 18:04
@bpkroth bpkroth disabled auto-merge November 15, 2024 18:59
@bpkroth bpkroth merged commit d408ab9 into microsoft:main Nov 15, 2024
16 of 17 checks passed
@bpkroth bpkroth deleted the doc-fixups branch November 15, 2024 18:59
bpkroth added a commit that referenced this pull request Nov 21, 2024
# Pull Request

## Title

Fixup broken markdown links after #869 

---

## Description

After #869 was merged the published documentation links changed which
meant that future Markdown lint checks failed.

This PR fixes that. 

---

## Type of Change

- 🛠️ Bug fix
- 📝 Documentation update

---
@bpkroth bpkroth mentioned this pull request Nov 22, 2024
4 tasks
bpkroth added a commit that referenced this pull request Dec 5, 2024
# Pull Request

## Title

More documentation updates

---

## Description

- [x] Adds general documentation on philosophy and usage
- [x] Additional docs about Config structure
- [x] Adjusts README documentation links
- [x] Additional documentation about Storage classes

---

## Type of Change

- 📝 Documentation update
- 🧪 Tests

---

## Additional Notes (optional)

Follow on to #869

---
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working ci documentation Improvements or additions to documentation ready for review Ready for review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants