Skip to content
This repository has been archived by the owner on Dec 18, 2023. It is now read-only.

Docs/evaluator pages #253

Merged
merged 28 commits into from Nov 22, 2022
Merged

Docs/evaluator pages #253

merged 28 commits into from Nov 22, 2022

Conversation

fabrizio-credo
Copy link
Contributor

Added evaluator schema + evaluators pages

  • Restructured the docs directory to improve tidyness
  • Evaluator schema/how to: docs/pages/evaluiators/make_your_own.rst
  • Each evaluator has an autogenerated page, code for autogeneration: docs/autogeneration/pages/evaluators.py

@esherman-credo
Copy link
Contributor

esherman-credo commented Nov 18, 2022

Docs currently failing to build https://readthedocs.org/projects/credoai-lens/builds/18676291/
(Read the docs appears to be on GMT? It says 7:40pm but the page above that shows it as a few minutes ago)

@fabrizio-credo
Copy link
Contributor Author

@esherman-credo General comment: the autogeneration of evaluator pages is disables, since it adds Lens dependency to RTD.
For now it needs to be run locally.

@esherman-credo
Copy link
Contributor

@esherman-credo General comment: the autogeneration of evaluator pages is disables, since it adds Lens dependency to RTD. For now it needs to be run locally.

Noted

@esherman-credo
Copy link
Contributor

It'd be nice if this page had links to each of the evaluator pages. E.g "DataEquity" is a link to here.

Is there a way to capitalize all words in multi-title pages (or keep the camelcase from the name of the evaluator in-code)?
image

@fabrizio-credo
Copy link
Contributor Author

It'd be nice if this page had links to each of the evaluator pages. E.g "DataEquity" is a link to here.

Is there a way to capitalize all words in multi-title pages (or keep the camelcase from the name of the evaluator in-code)?

Done!

Copy link
Contributor

@esherman-credo esherman-credo left a comment

Choose a reason for hiding this comment

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

Some minor changes throughout.

Some items are beyond the scope of this review. Please make Jira tickets as appropriate so that we can follow-up (e.g. fixing evaluator docstrings, adopting an enforced documentation protocol, etc.).

credoai/evaluators/equity.py Outdated Show resolved Hide resolved
credoai/evaluators/equity.py Show resolved Hide resolved
credoai/evaluators/evaluator.py Outdated Show resolved Hide resolved
credoai/evaluators/evaluator.py Show resolved Hide resolved
credoai/evaluators/evaluator.py Outdated Show resolved Hide resolved
credoai/evaluators/survival_fairness.py Show resolved Hide resolved
credoai/governance/__init__.py Outdated Show resolved Hide resolved
After pushing to github, Read the Docs will build the documentation if the
branch is currently activated.

For local build, on top of the previous commands, run `make html`directory and the docs site will build to: `docs/_build/html/index.html`, which can be opened in the browser.
Copy link
Contributor

Choose a reason for hiding this comment

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

Should add instructions for what to do if you can't auto-build, since that's the current state of things.

What is the process for putting locally-built docs onto RTD manually?

What is necessary to go back to a state where we can auto-generate/auto-push to RTD?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Should add instructions for what to do if you can't auto-build, since that's the current state of things.

@esherman-credo do you mean in case RTD build fails?

What is the process for putting locally-built docs onto RTD manually?

You mean loading the local build directly to RTD? I don't think there is a way to do that. If the build works locally, in general it should work remotely, bar dependency issues. There shouldn't be a need for this.

What is necessary to go back to a state where we can auto-generate/auto-push to RTD?

It would be necessary to add Lens as a dependency, which is pretty onerous for just a doc build. This is due
to the code in the evaluators page generator being dependent on build_list_of_evaluators and the internal structure of the Lens package. We could find maybe some workaround, atm I think we are OK with the current manual approach.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah forgive me. I was under the impression something wasn't building on RTD and that you had to upload manually. Based on discussion last week Wed or Fri.

It seems by "manual" you mean parsing the files rather than importing?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah running the parsing manually before pushing to github.

docs/autogeneration/formatter.py Show resolved Hide resolved
docs/index.rst Outdated Show resolved Hide resolved
@fabrizio-credo fabrizio-credo merged commit fe82be1 into develop Nov 22, 2022
@fabrizio-credo fabrizio-credo deleted the docs/evaluator_pages branch November 22, 2022 20:23
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants