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

Refactor getter functions #430

Merged
merged 5 commits into from
Feb 1, 2023
Merged

Refactor getter functions #430

merged 5 commits into from
Feb 1, 2023

Conversation

janosg
Copy link
Member

@janosg janosg commented Jan 31, 2023

Problem Description

Tranquilo consists of many interchangeable components. Usually a component is a partialled function (provided by the user or loaded from a dictionary of built in functions). Sometimes, not all supported functions have the same signature and the processed function needs to ignore unused arguments.

Solution

Write a get_component function that covers most of our use-cases.

@codecov
Copy link

codecov bot commented Jan 31, 2023

Codecov Report

Merging #430 (937587c) into main (0f252a1) will increase coverage by 0.11%.
The diff coverage is 98.47%.

@@            Coverage Diff             @@
##             main     #430      +/-   ##
==========================================
+ Coverage   92.73%   92.84%   +0.11%     
==========================================
  Files         229      231       +2     
  Lines       17522    17590      +68     
==========================================
+ Hits        16249    16332      +83     
+ Misses       1273     1258      -15     
Impacted Files Coverage Δ
tests/optimization/tranquilo/test_get_component.py 98.00% <98.00%> (ø)
.../estimagic/optimization/tranquilo/get_component.py 98.36% <98.36%> (ø)
.../estimagic/optimization/tranquilo/filter_points.py 83.14% <100.00%> (+2.71%) ⬆️
src/estimagic/optimization/tranquilo/fit_models.py 84.93% <100.00%> (+2.89%) ⬆️
.../estimagic/optimization/tranquilo/sample_points.py 96.80% <100.00%> (+3.74%) ⬆️
src/estimagic/optimization/tranquilo/tranquilo.py 96.51% <100.00%> (ø)
tests/optimization/tranquilo/test_filter_points.py 100.00% <100.00%> (ø)

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

@janosg janosg requested a review from timmens January 31, 2023 15:59
Copy link
Member

@timmens timmens left a comment

Choose a reason for hiding this comment

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

Very nice PR. I like the test cases and their simplicity. It's much better to have this functionality outside the individual modules.

Approved already; I only have one question:

  1. Regarding the comments I made in the code: Is the design choice that default_options and mandatory_args have a non-empty intersection deliberate? As a reader, I would've expected that arguments are not options and vice versa.

@janosg janosg merged commit 66ac796 into main Feb 1, 2023
@janosg janosg deleted the refactor-getters branch February 1, 2023 10:33
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.

2 participants