-
Notifications
You must be signed in to change notification settings - Fork 39
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
Test metrics #140
Test metrics #140
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.
api/tests/data/detections_heatmap_2020-09-19_EXAMPLE.npy is an empty file. Maybe you want to remove it?
@@ -45,8 +45,8 @@ def get_area_occupancy_daily_report(areas: str = "", | |||
def get_area_occupancy_weekly_report( | |||
areas: str = "", | |||
weeks: int = Query(0), | |||
from_date: date = Query((date.today() - timedelta(days=date.today().weekday(), weeks=4)).isoformat()), |
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.
Why are we removing this?
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.
Because this causes several bugs.
First, when you do not send neither from_date nor the to_date as a query parameter, default values will take place and will be sent to the corresponding report function. However, in both cases, a string will be sent, not a date. (Important: .isoformat()
returns an string).
So, for example, when get_weekly_metric
is reached and we have to set a week_span:
if number_of_days > 0:
# Separate weeks in range taking a number of weeks ago, considering the week ended yesterday
date_range = pd.date_range(end=date.today() - timedelta(days=1), periods=number_of_days)
start_dates = date_range[0::7]
end_dates = date_range[6::7]
week_span = list(zip(start_dates, end_dates))
elif isinstance(from_date, date) and isinstance(to_date, date):
# Separate weeks in range considering the week starts on Monday
date_range = pd.date_range(start=from_date, end=to_date)
week_span = list(parse_date_range(date_range))
else:
week_span = []
Given range of dates are ok, but are both strings, so will never enter elif isinstance(from_date, date) and isinstance(to_date, date):
and week_span will be an empty array, and this, should not be like that. As a result, no data will be returned on the endpoint, regardless of whether or not there is.
Secondly, there was another issue that appears when you submit only one the two dates a let the other one be the default value. As we explained before, default values will be string, while submitted dates are type: "date". So when the code reaches:
def validate_dates(from_date: date, to_date: date):
if from_date > to_date:
raise HTTPException(
status_code=status.HTTP_400_BAD_REQUEST,
detail=bad_request_serializer(
"Invalid range of dates",
error_type="from_date doesn't come before to_date",
loc=["query", "from_date"]
)
)
An exception will be thrown because we cannot compare a string with a date.
In this branch, tests for the following endpoints were added:
/metrics/cameras/{camera_id}/heatmap
/metrics/cameras/social-distancing/live
/metrics/cameras/social-distancing/hourly
/metrics/cameras/social-distancing/daily
/metrics/cameras/social-distancing/weekly
/metrics/cameras/face-mask-detections/live
/metrics/cameras/face-mask-detections/hourly
/metrics/cameras/face-mask-detections/daily
/metrics/cameras/face-mask-detections/weekly
/metrics/areas/social-distancing/live
/metrics/areas/social-distancing/hourly
/metrics/areas/social-distancing/daily
/metrics/areas/social-distancing/weekly
/metrics/areas/face-mask-detections/live
/metrics/areas/face-mask-detections/hourly
/metrics/areas/face-mask-detections/daily
/metrics/areas/face-mask-detections/weekly
/metrics/areas/occupancy/live
/metrics/areas/occupancy/hourly
/metrics/areas/occupancy/daily
/metrics/areas/occupancy/weekly
Note: there are some comments with the tag
#TODO
that should be ignored for now.