Adds Stat class and calculates additional statistics - #120
Conversation
| elif not delta.is_unmodified(): | ||
| yield delta.to_dict() | ||
|
|
||
| def stats(deltacode): |
There was a problem hiding this comment.
No need for this function if you are just going to run deltacode.stats.percent_to_dict()
There was a problem hiding this comment.
@MaJuRG so from the cli.py file I directly call the stats.to_dict() method?
| self.removed = 0 | ||
| self.moved = 0 | ||
| self.modified = 0 | ||
| self.unmodified = 0 |
There was a problem hiding this comment.
these field names should be prefixed with num_
| self.removed_percent = 0 | ||
| self.moved_percent = 0 | ||
| self.modified_percent = 0 | ||
| self.unmodified_percent = 0 |
There was a problem hiding this comment.
you should switch these field names around: percent_*
| self.modified_percent = percent(self.modified, self.old_files_count) | ||
| self.unmodified_percent = percent(self.unmodified, self.old_files_count) | ||
|
|
||
| def percent_to_dict(self): |
There was a problem hiding this comment.
Probably rename this to simply to_dict, since this is a Stat object.
There was a problem hiding this comment.
@MaJuRG I named it as percent_to_dict because we currently need only percent stats, in future if we want to get all the data variables of this class then we can use another method to_dict() for that case. Should I rename this to to_dict()?
There was a problem hiding this comment.
Yes, I would rename this.
| self.removed_percent = percent(self.removed, self.old_files_count) | ||
| self.moved_percent = percent(self.moved, self.old_files_count) | ||
| self.modified_percent = percent(self.modified, self.old_files_count) | ||
| self.unmodified_percent = percent(self.unmodified, self.old_files_count) |
There was a problem hiding this comment.
Why don't we just do the math at this level instead of calling percent(x, y)
There was a problem hiding this comment.
@MaJuRG since we need to calculate percent, again and again, I made a util function, such that we can use this in the future as well if we need it. Should I calculate here only and delete the percent function?
There was a problem hiding this comment.
@arnav-mandal1234 Yes, you are right, that makes sense to me. You should probably rename it calculate_percent though
Adds Stat class to __init__.py and calculates various percent stats Fixes : #5 Signed-off-by: arnav-mandal1234 <arnav.mandal1234@gmail.com>
2 participants
Adds Stat class to init.py and calculates various percent stats
Fixes : #5
Signed-off-by: arnav-mandal1234 arnav.mandal1234@gmail.com