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

test run example notebooks #613

Merged
merged 14 commits into from
Sep 6, 2024

Conversation

juanitorduz
Copy link
Contributor

Closes #606

Copy link

codecov bot commented Sep 4, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

see 27 files with indirect coverage changes

@juanitorduz juanitorduz marked this pull request as draft September 4, 2024 18:04
DOCS / "marginaleffects.ipynb",
DOCS / "quickstart.ipynb",
DOCS / "replicating-the-effect.ipynb",
DOCS / "stargazer.ipynb", # failing notebook
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@s3alfisc in Cell[22] there is an error in the notebook (checked locally)

NotImplementedError: <class 'pyfixest.estimation.feols_.Feols'>

See https://github.com/py-econometrics/pyfixest/actions/runs/10707033524/job/29686144381

KERNEL_NAME: str = "python3"
DOCS = Path("docs")
NOTEBOOKS: list[Path] = [
# DOCS / "compare-fixest-pyfixest.ipynb", # needs R
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Let me work on this notebook on a different PR as it requires some investigation.

Copy link
Member

Choose a reason for hiding this comment

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

The error likely stems from the fact that Stargazer's default does not have pyfixest support - makes sense to handle this in another pr!

@juanitorduz juanitorduz marked this pull request as ready for review September 4, 2024 18:26
@juanitorduz
Copy link
Contributor Author

@s3alfisc this is ready for review. This set up the tests. I commented two notebooks:

@juanitorduz juanitorduz requested a review from s3alfisc September 6, 2024 07:53
@juanitorduz
Copy link
Contributor Author

ok! so any suggested changes or are we good to go @s3alfisc ? :)

Copy link
Member

@s3alfisc s3alfisc 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 to me, will merge now. Sorry that I did not finish the PR review yesterday, I got interrupted and then did not manage to get back to it. Btw I think you are successfully installing R via r2u, so the notebook depending on rpy2/fixest should execute properly =) thank you!

@s3alfisc s3alfisc merged commit addd3cb into py-econometrics:master Sep 6, 2024
8 checks passed
@juanitorduz juanitorduz deleted the test_notebooks branch September 6, 2024 08:29
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.

CI: Add workflow to automatically render all Example Notebooks
2 participants