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

Code duplication within eval functionality & unittests #199

Open
mstites opened this issue Jul 29, 2022 · 0 comments
Open

Code duplication within eval functionality & unittests #199

mstites opened this issue Jul 29, 2022 · 0 comments
Labels
good first issue Good for newcomers

Comments

@mstites
Copy link
Collaborator

mstites commented Jul 29, 2022

Throughout grama, there is currently a lot of redudent code that could be combined into common functionality. De-duplicated code would improve readability and maintainability.

From my work so far, I have identified two major areas to start work on code de-duplication within eval verbs:

  1. Unittesting eval verbs: both test_evals.py and test_evals.py have a lot of repeating code. For example, test_lhs and test_sample follow almost identical tests, except for the specific df arg and kwargs. These tests could be combined into a helper function that takes a specific df arg (for input and for testing against(?)) and passes kwargs. I created the class TestEvalInvariants which groups invariaant unittests for eval verbs*. Similar code/classes could be developed for other tests.
  2. Eval verbs invariant testing: PR Dev tuple trap - grouping of common invariant tests for eval verbs #195 combined invariant tests into a centralized helper function. However, this invariant testing currently is performed twice for any eval_* verb that calls eval_df as part of its operation (e.g. eval_nominal, eval_conservative). This is necessary with the way the code is currently written, as invariants must be tested before any operation is done on the model or dataframe objects (these eval_* verbs plus eval_df are all user accessible functions). A potential way to remove this code duplication would be to create a helper function that performs the necessary part of eval_df these functions require, so that these functions can call an internal helper function (w/o invariant tests) instead of eval_df.

Other areas of the code also need work in deduplication, and other issues should be opened (or this one modified) to cover those. A good place to start would just be working on de-duplication throughout the test suite.

*This class is a good start, however, there is quite a bit of code duplication here between the df_arg and df_arg_2 functions. These could also be combined to reduce code duplication.

@mstites mstites added the good first issue Good for newcomers label Jul 29, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers
Projects
None yet
Development

No branches or pull requests

1 participant