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

initial commit for aka.ms/econml doc migration #640

Merged
merged 11 commits into from
Jul 27, 2022
Merged

Conversation

fverac
Copy link
Collaborator

@fverac fverac commented Jun 22, 2022

Currently there is some useful documentation on the Microsoft EconML project page but it can be hard to find. Migrating the information over to the package docs should increase visibility.

@fverac fverac force-pushed the fverac/migrate_akamseconml branch from 83c8d66 to 7055354 Compare July 13, 2022 18:56
@fverac fverac marked this pull request as ready for review July 14, 2022 15:05
Copy link
Collaborator

@vsyrgkanis vsyrgkanis left a comment

Choose a reason for hiding this comment

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

Looks good! Maybe only I would try to place the three figures in the main page of our econml project webpage, on flexbiilit ..., in the intro page of the docs.

Copy link
Collaborator

@kbattocchi kbattocchi left a comment

Choose a reason for hiding this comment

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

Looks great! Added one minor comment about image hosting.

<div class="ms-grid " style = "text-align: left; box-sizing: border-box; display: block; margin-left: auto; margin-right: auto; max-width: 1600px; position: relative; padding-left: 0; padding-right: 0; width: 100%;">
<div class="ms-row" style = "text-align: left; box-sizing: border-box; -webkit-box-align: stretch; align-items: stretch; display: flex; flex-wrap: wrap; margin-left: 3px; margin-right: 3px;">
<div class="m-col-8-24 x-hidden-focus" style = "text-align: left; box-sizing: border-box; float: left; margin: 0; padding-left: 1vw; padding-right: 1vw; position: relative; width: 33.33333%;">
<p style="text-align:center;"><img loading="lazy" class="size-full wp-image-656358 aligncenter x-hidden-focus" src="https://www.microsoft.com/en-us/research/uploads/prod/2020/05/imgFlexible.png" alt="Flexible icon" width="92" height="92"></p><p style="text-align: center"><b>Flexible</b></p><p class="x-hidden-focus">Allows for flexible model forms that do not impose strong assumptions, including models of heterogenous responses to treatment.</p><p> </p></div>
Copy link
Collaborator

Choose a reason for hiding this comment

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

It might be better to copy the images locally rather than accessing them from www.microsoft.com.

@@ -10,7 +10,6 @@ __pycache__/
*.log
*.out
*.synctex.gz
*.pdf
Copy link
Collaborator

Choose a reason for hiding this comment

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

This change is fine.

If you need to make any further changes to this PR, you might consider also changing azure-pipelines.yml to exempt .gitignore changes so that you don't have to run the full set of checks, since all of your other changes are in the doc subdirectory.

Copy link
Collaborator Author

@fverac fverac left a comment

Choose a reason for hiding this comment

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

sphinx-doc/sphinx#10705
Because of issues with sphinx 5.1.0

@@ -71,7 +71,7 @@ jobs:
- script: 'pip install git+https://github.com/slundberg/shap.git@d1d2700acc0259f211934373826d5ff71ad514de'
displayName: 'Install specific version of shap'

- script: 'pip install sphinx sphinx_rtd_theme'
- script: 'pip install sphinx!=5.1.0 sphinx_rtd_theme'
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

^

@fverac fverac merged commit b4832c0 into main Jul 27, 2022
@fverac fverac deleted the fverac/migrate_akamseconml branch July 27, 2022 15:07
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.

None yet

3 participants