Added CourseDailyMetrics #13
Conversation
johnbaldwin
commented
Apr 18, 2018
johnbaldwin
commented
- Added the model, serializer, filter, view and tests for each
- Added CourseDailyMetrics to the admin interface
- Added model - Added migration - Added test fixture and basic model test
- Added an is_close helper method to compare floating point values. See the method docstring for more information
- Added course_id to test data - Added new test to check unique together constraint - Added placeholder tests for future implementation
- Also added 'create_metrics_model_timeseries' method to tests/helpers
|
@johnbaldwin Would you mind squashing the commits and pushing again? |
Codecov Report
@@ Coverage Diff @@
## john/site-daily-metrics #13 +/- ##
==========================================================
Coverage ? 93.64%
==========================================================
Files ? 25
Lines ? 614
Branches ? 0
==========================================================
Hits ? 575
Misses ? 39
Partials ? 0
Continue to review full report at Codecov.
|
| # Any other data we want? | ||
|
|
||
| def __str__(self): | ||
| return "id:{}, date_for:{}, course_id:{}".format( |
There was a problem hiding this comment.
Doesn't look like __str__ returns unicode, and you're using @python2_unicode_compatible. Same on next method. Don't you need to return u"id:{}...?
There was a problem hiding this comment.
@bryanlandia Good find. The docs are weak. They should specify that str needs to be defined as unicode. See: https://github.com/django/django/blob/stable/1.8.x/django/utils/encoding.py#L28
For ref, here the doc for the decorator:
https://docs.djangoproject.com/en/1.8/ref/utils/#module-django.utils.encoding
So I'll create an issue to fix this globally after we've got these PRs merged to develop
There was a problem hiding this comment.
I created an issue to track that this needs to get fixed: #18
…upgrade-2 Merge Appsembler master changes to 0.3.19
