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

Improve infrastructure (CI, Linting, Docs Build) #420

Merged
merged 42 commits into from
Jan 14, 2023
Merged

Conversation

hmgaudecker
Copy link
Member

If there is need for speed.

@codecov
Copy link

codecov bot commented Dec 22, 2022

Codecov Report

Merging #420 (19141f2) into main (79e8a0e) will decrease coverage by 0.06%.
The diff coverage is 87.55%.

@@            Coverage Diff             @@
##             main     #420      +/-   ##
==========================================
- Coverage   92.88%   92.82%   -0.07%     
==========================================
  Files         223      223              
  Lines       17396    17459      +63     
==========================================
+ Hits        16158    16206      +48     
- Misses       1238     1253      +15     
Impacted Files Coverage Δ
src/estimagic/batch_evaluators.py 60.29% <0.00%> (ø)
src/estimagic/dashboard/callbacks.py 59.09% <ø> (ø)
src/estimagic/optimization/neldermead.py 90.62% <ø> (-0.10%) ⬇️
src/estimagic/optimization/nlopt_optimizers.py 92.66% <ø> (ø)
src/estimagic/optimization/scipy_optimizers.py 99.04% <ø> (ø)
src/estimagic/optimization/subsolvers/_trsbox.py 84.95% <ø> (ø)
...timagic/optimization/tranquilo/aggregate_models.py 87.71% <ø> (ø)
src/estimagic/parameters/check_constraints.py 76.41% <ø> (ø)
src/estimagic/parameters/kernel_transformations.py 99.20% <ø> (-0.01%) ⬇️
src/estimagic/parameters/nonlinear_constraints.py 87.93% <0.00%> (ø)
... and 73 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@review-notebook-app
Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

@hmgaudecker
Copy link
Member Author

Broadening the scope a bit...

@hmgaudecker hmgaudecker requested a review from janosg December 22, 2022 14:35
@hmgaudecker
Copy link
Member Author

Some economies of scope with the review of GETTSIM #470 here -- exactly the same 😄

@janosg janosg changed the title Use mamba in readthedocs. Improve infrastructure (CI, Linting, Docs Build) Jan 13, 2023
@janosg
Copy link
Member

janosg commented Jan 13, 2023

Runtime of github actions is roughly cut in half due to mamba and caching environments. I also got rid of tox entirely. @hmgaudecker maybe you want to copy this for gettsim.

@hmgaudecker
Copy link
Member Author

Thanks @janosg ! Will copy, but for doing so common formatting of yaml files is really helpful. I added yamlfix and yamllint hooks, but was unable to push because @timmens was always faster than me and things interacted with his changes.

Tim, could you try to merge the yamlfix branch into this?

@janosg
Copy link
Member

janosg commented Jan 14, 2023

Turns out that the notebooks were not actually executed on readthedocs and shown without outputs, due to this line in conf.py:

nbsphinx_execute = "never"

Unfortunately, they also run way too long to be executed remotely. So I keep the mamba changes but otherwise revert to building notebooks locally and committing them with outputs.

@hmgaudecker
Copy link
Member Author

Do all notebooks run for too long? Else use "auto" and commit only outputs of those with long runtimes?

@timmens timmens merged commit 608dc5c into main Jan 14, 2023
@timmens timmens deleted the use-mamba-on-rtd branch January 14, 2023 18:46
@hmgaudecker hmgaudecker mentioned this pull request Jan 18, 2023
2 tasks
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.

4 participants