-
Notifications
You must be signed in to change notification settings - Fork 2
OPTIONAL - Colored error indication #140
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
base: main
Are you sure you want to change the base?
Conversation
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.
Pull Request Overview
A concise summary of changes to improve numeric formatting, clean up debug logs, and enhance pagination utilities.
- Removed a stray
console.logfrom theHeadercomponent. - Standardized numeric display by switching to
toPrecision(3)inMetricandJobRow. - Introduced a default limit in
_get_all_itemsand added atest_job_retrieval_paginatedhelper.
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| node/src/components/navigation/Header.tsx | Removed leftover console.log debugging output |
| node/src/components/Metric.tsx | Changed formatting from toFixed(4) to toPrecision(3) |
| node/src/components/JobRow.tsx | Replaced toExponential(3) with toPrecision(3) |
| flaskapi/flask_workflows.py | Added default pagination limit and a standalone pagination test helper |
Comments suppressed due to low confidence (1)
flaskapi/flask_workflows.py:367
- This test helper is embedded in production code and only logs output; consider moving it to a dedicated test file with assertions for automated validation.
def test_job_retrieval_paginated(function_uid: str):
| """Helper function to get all items from a paginated API call.""" | ||
| list_len = api_call(limit=1,*args, **kwargs).total | ||
| if "limit" not in kwargs: | ||
| kwargs["limit"] = int(np.min([50, list_len])) ## max allowed is 50 |
Copilot
AI
Jun 21, 2025
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.
Using NumPy's np.min for comparing two scalar values adds an external dependency; consider using the built-in min(50, list_len) for simplicity and reduced overhead.
| def _timeit(fun: Callable, *args, **kwargs): | ||
| import time | ||
| start_time = time.time() | ||
| result = fun(*args, **kwargs) | ||
| end_time = time.time() | ||
| _logger.info(f"Retrieved {len(result)} items in {end_time - start_time:.4f} seconds") | ||
| _logger.info(f"First item: {result[0] if result else 'No items retrieved'}") | ||
| _logger.info(f"Last item: {result[-1] if result else 'No items retrieved'}") | ||
| _logger.info(f"That is {(end_time - start_time)/len(result):.2f} seconds per item") |
Copilot
AI
Jun 21, 2025
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.
The nested _timeit duplicates the existing top-level helper; reuse the original function to avoid code duplication and ensure consistency.
| def _timeit(fun: Callable, *args, **kwargs): | |
| import time | |
| start_time = time.time() | |
| result = fun(*args, **kwargs) | |
| end_time = time.time() | |
| _logger.info(f"Retrieved {len(result)} items in {end_time - start_time:.4f} seconds") | |
| _logger.info(f"First item: {result[0] if result else 'No items retrieved'}") | |
| _logger.info(f"Last item: {result[-1] if result else 'No items retrieved'}") | |
| _logger.info(f"That is {(end_time - start_time)/len(result):.2f} seconds per item") | |
| def log_callback(result, *args, **kwargs): | |
| _logger.info(f"Retrieved {len(result)} items in {kwargs['execution_time']:.4f} seconds") | |
| _logger.info(f"First item: {result[0] if result else 'No items retrieved'}") | |
| _logger.info(f"Last item: {result[-1] if result else 'No items retrieved'}") | |
| _logger.info(f"That is {kwargs['execution_time']/len(result):.2f} seconds per item") | |
| _timeit(_get_all_items, api_call=functions_api_instance.list_function_jobs_for_functionid, function_id=function_uid, log_callback=log_callback) # type: ignore |
DO NOT MERGE - to be discussed with Esra (both idea & threshold values for the colors) and Alex (which colors to use; how to be more clear, as the mean & std to the left already have colors)
Made error metrics (MAE, RMSE) show colored depending on the quality-of-fit:
Ok vs bad example:
