Skip to content

Conversation

@teubert
Copy link
Collaborator

@teubert teubert commented Jun 14, 2023

A few suggested changes:

  • Remove duplicate import of warn
  • Reorder imports to be in alphabetical (reduces likelihood of future duplicate imports)
  • Move to using collections.abc.Callable (typing.Callable deprecated by python)
  • Fix delitem (super had extra argument)
  • Remove double drop from .pop (was dropped in pop and also pop_by_index, caused error).
  • Newline between methods

@teubert teubert requested a review from mstraut June 14, 2023 18:57
@github-actions
Copy link

Benchmarking Results
From:

Test Time (s)
import main 0.11410330000000002
import thrown object 0.4917951999999999
model initialization 0.1529940999999999
set noise 0.6696369
simulate 0.4903369999999998
simulate with saving 1.3959897
simulate with saving, dt 1.7792702
simulate with printing results, dt 2.223371
Plot results 14.210425500000001
Metrics 0.039763999999998134
Surrogate Model Generation 2.1584778
surrogate sim 1.4123605999999995
surrogate sim, dt 3.709146200000003
To:
Test Time (s)
--- ---
import main 0.11978880000000003
import thrown object 0.5035624999999999
model initialization 0.15466970000000035
set noise 0.6784387999999999
simulate 0.4945919000000001
simulate with saving 1.3655130999999998
simulate with saving, dt 1.7831238000000003
simulate with printing results, dt 2.218001199999999
Plot results 14.231689000000001
Metrics 0.040882299999999816
Surrogate Model Generation 2.1593595000000008
surrogate sim 1.4044836000000025
surrogate sim, dt 3.6855575000000016

@codecov-commenter
Copy link

Codecov Report

Merging #554 (07cd465) into prog_models_feature/containers_simresult_tests_updates (b3ad9b0) will increase coverage by 0.05%.
The diff coverage is 75.00%.

@@                                    Coverage Diff                                     @@
##           prog_models_feature/containers_simresult_tests_updates     #554      +/-   ##
==========================================================================================
+ Coverage                                                   84.35%   84.40%   +0.05%     
==========================================================================================
  Files                                                          34       34              
  Lines                                                        2582     2578       -4     
==========================================================================================
- Hits                                                         2178     2176       -2     
+ Misses                                                        404      402       -2     
Impacted Files Coverage Δ
src/prog_models/sim_result.py 87.77% <75.00%> (+0.82%) ⬆️

Copy link
Contributor

@mstraut mstraut left a comment

Choose a reason for hiding this comment

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

Thanks for looking this over.

@mstraut mstraut merged commit 2c30b9a into prog_models_feature/containers_simresult_tests_updates Jun 14, 2023
@mstraut mstraut deleted the simresult_suggestions branch June 14, 2023 19:39
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