-
Notifications
You must be signed in to change notification settings - Fork 120
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
Feat/check res table #848
Feat/check res table #848
Conversation
Codecov Report
@@ Coverage Diff @@
## main #848 +/- ##
==========================================
- Coverage 58.08% 58.02% -0.07%
==========================================
Files 172 172
Lines 10560 10573 +13
==========================================
+ Hits 6134 6135 +1
- Misses 4426 4438 +12 |
|
||
|
||
# Function to build main | ||
def check_res_table(row_names=None, simulation_res=None, target_res=None): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add docstring here.
# If someone did not identify inputs at all, | ||
# it represents default data frame example. | ||
if row_names is None: | ||
print("No Inputs") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Don't print, raise:
raise ValueError('missing row_names')
|
||
|
||
# Function to build main | ||
def check_res_table(row_names=None, simulation_res=None, target_res=None): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If these parameters are always required, then should be using arg and not kwarg.
if len(row_names) != len(target_res) or len(row_names) != len(target_res): | ||
raise TypeError("Not the same quantity of the rows and filled data") | ||
|
||
# Import Pandas and Numpy. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Move to top, specify why you're doing a lazy import.
Recommending abandoning this due to changes in |
This PR takes into account the necessity to create a data frame for printing output results on the VMs using Pandas.
It takes in account issue: #846