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.

Sign up for GitHub

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

Jump to bottom

Fix the Warnings in rst files when building Qlib's documentation #1349

Merged
merged 17 commits into from
Nov 13, 2022
Merged

Fix the Warnings in rst files when building Qlib's documentation #1349

merged 17 commits into from
Nov 13, 2022

Conversation

MaximSmolskiy
Copy link
Contributor

@MaximSmolskiy MaximSmolskiy commented Nov 12, 2022 •
edited
Loading

Fix all warnings in rst files.

Description

Motivation and Context

Related to #1345.

How Has This Been Tested?

  • Pass the test by running: pytest qlib/tests/test_all_pipeline.py under upper directory of qlib.
  • If you are adding a new feature, test on your own test scripts.

Screenshots of Test Results (if appropriate):

  1. Pipeline test:
  2. Your own tests:

Types of changes

  • Fix bugs
  • Add new feature
  • Update documentation

@MaximSmolskiy
Copy link
Contributor Author

@microsoft-github-policy-service agree

you-n-g
you-n-g reviewed Nov 13, 2022
View reviewed changes
docs/advanced/task_management.rst
@@ -18,7 +18,7 @@ With this module, users can run their ``task`` automatically at different period

This whole process can be used in `Online Serving <../component/online.html>`_.

An example of the entire process is shown `here <https://github.com/microsoft/qlib/tree/main/examples/model_rolling/task_manager_rolling.py>`_.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why two underscores are necessary here?

I didn't find any reference in rst docs

Could you please share me any related references?
Thanks

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Previous version (with one underscore) raises WARNING: Duplicate explicit target name: "..."., I found the solution here

Copy link
Collaborator

Choose a reason for hiding this comment

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

@MaximSmolskiy
It really helpful to me.
Thanks!

@you-n-g
Copy link
Collaborator

you-n-g commented Nov 13, 2022 •
edited
Loading

It still reports a lot of warnings in my environment.
image

But the current PR has already fixed a lot of issues is really helpful.
I can merge it first if you want to fix it in other PRs in the future.

@MaximSmolskiy
Copy link
Contributor Author

It still reports a lot of warnings in my environment. image

But the current PR has already fixed a lot of issues is really helpful. I can merge it first you want to fix it in other PRs in the future.

Yes, in this PR I wanted to fix all warnings in rst files. Now there should be only 4 warnings with rst files:
WARNING: document isn't included in any toctree. I'm not sure if they need to be added, and if yes, that I can do it well (add them in index.rst in proper places with proper titles).

The the PR turned out to be not small (about hundred lines were changed).

In the future I will try to fix other errors/warnings in other PRs.

@you-n-g
Copy link
Collaborator

you-n-g commented Nov 13, 2022

I think docs/developer/code_standard_and_dev_guide.rst should be included in the index.rst

Files in hidden (docs/hidden/client.rst, docs/hidden/online.rst and docs/hidden/tuner.rst) should be ignored.
Maybe exclude_patterns will be useful

@you-n-g
Copy link
Collaborator

you-n-g commented Nov 13, 2022

I think docs/developer/code_standard_and_dev_guide.rst should be included in the index.rst

Files in hidden (docs/hidden/client.rst, docs/hidden/online.rst and docs/hidden/tuner.rst) should be ignored. Maybe exclude_patterns will be useful

Would you like to update it in this PR?
Otherwise I'll merge it first.

@MaximSmolskiy
Copy link
Contributor Author

I think docs/developer/code_standard_and_dev_guide.rst should be included in the index.rst
Files in hidden (docs/hidden/client.rst, docs/hidden/online.rst and docs/hidden/tuner.rst) should be ignored. Maybe exclude_patterns will be useful

Would you like to update it in this PR? Otherwise I'll merge it first.

I will try to do it in this PR soon

@MaximSmolskiy
Copy link
Contributor Author

@you-n-g Now there are no warnings left with rst files and updated toc looks as follows
Снимок экрана от 2022-11-13 16-42-22

@you-n-g
Copy link
Collaborator

you-n-g commented Nov 13, 2022 •
edited
Loading

It looks great.
Do you think it will be more reasonable to put it before REFERENCE: and after OTHER COMPONENTS/FEATURES/TOPICS:?
@MaximSmolskiy

@MaximSmolskiy MaximSmolskiy force-pushed the fix-the-Errors/Warnings-when-building-Qlibs-documentation branch from 4a3f228 to 7f221a5 Compare November 13, 2022 14:01
you-n-g
you-n-g reviewed Nov 13, 2022
View reviewed changes
docs/index.rst Outdated
@@ -30,6 +30,11 @@ Document Structure
Data Retrieval <start/getdata.rst>
Custom Model Integration <start/integration.rst>

.. toctree::
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do you think it will be more reasonable to put it before REFERENCE: and after OTHER COMPONENTS/FEATURES/TOPICS:?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

@you-n-g you-n-g merged commit 82afd6a into microsoft:main Nov 13, 2022
@you-n-g
Copy link
Collaborator

you-n-g commented Nov 13, 2022

It looks great!
Thanks for your great efforts.
Welcome to becoming one of Qlib's contributors!

@MaximSmolskiy MaximSmolskiy deleted the fix-the-Errors/Warnings-when-building-Qlibs-documentation branch November 13, 2022 14:30
@you-n-g you-n-g added the documentation Improvements or additions to documentation label Dec 9, 2022
qianyun210603 pushed a commit to qianyun210603/qlib that referenced this pull request Mar 23, 2023
@MaximSmolskiy
Fix the Warnings in rst files when building Qlib's documentation (mic…
5e2b776 
…rosoft#1349)

* Fix docs/advanced/alpha.rst

* Fix docs/reference/api.rst

* Fix docs/component/strategy.rst

* Fix docs/start/integration.rst

* Fix docs/component/report.rst

* Fix docs/component/data.rst

* Fix docs/component/rl/framework.rst

* Fix docs/introduction/quick.rst

* Fix docs/advanced/task_management.rst

* Fix CHANGES.rst

* Fix docs/developer/code_standard_and_dev_guide.rst

* Fix docs/hidden/client.rst

* Fix docs/component/online.rst

* Fix docs/start/getdata.rst

* Add docs/hidden to exclude patterns

* Add docs/developer/code_standard_and_dev_guide.rst to index.rst

* Change docs/developer/code_standard_and_dev_guide.rst place in index.rst
qianyun210603 pushed a commit to qianyun210603/qlib that referenced this pull request Mar 23, 2023
@MaximSmolskiy
Fix the Warnings in rst files when building Qlib's documentation (mic…
c47cc31 
…rosoft#1349)

* Fix docs/advanced/alpha.rst

* Fix docs/reference/api.rst

* Fix docs/component/strategy.rst

* Fix docs/start/integration.rst

* Fix docs/component/report.rst

* Fix docs/component/data.rst

* Fix docs/component/rl/framework.rst

* Fix docs/introduction/quick.rst

* Fix docs/advanced/task_management.rst

* Fix CHANGES.rst

* Fix docs/developer/code_standard_and_dev_guide.rst

* Fix docs/hidden/client.rst

* Fix docs/component/online.rst

* Fix docs/start/getdata.rst

* Add docs/hidden to exclude patterns

* Add docs/developer/code_standard_and_dev_guide.rst to index.rst

* Change docs/developer/code_standard_and_dev_guide.rst place in index.rst
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@you-n-g you-n-g you-n-g left review comments

Assignees
No one assigned
Labels
documentation Improvements or additions to documentation
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@MaximSmolskiy @you-n-g