feat(medcat): Get Stats: harder, better, (probably not) faster, stronger - #606
adam-sutton-1992 wants to merge 15 commits into
Conversation
|
NOTE: |
mart-r
left a comment
There was a problem hiding this comment.
Overall, I think this is definitely a step in the right direction!
The setup looks good, the output seems easy to use.
And there's still the option to get the same sort of output.
I don't really think we can introduce a breaking change in the manner that you're doing it here due to unkown downstream effects.
So I'd say this thing (i.e the new returned object) needs to be in its own method and the get_stats needs to use this and unwrap the output (like you've done in various bits).
There's a few nagging things.
But also a few things that I think would need to change.
A few structures I'd like to be defined more rigidly (rather than just dict or predefined strings).
A few bits where I feel like we could easily split out the longer methods into smaller ones.
And then there's a matter of documentation in a few places.
And then one place where I asked for a feature for print output stream.
mart-r
left a comment
There was a problem hiding this comment.
A few doc strings I'd like to see, plus removal of added comments from test_kfold.py.
And the project + aggregate issue that's still present. Would be nice to clean that up, but I don't think it's high priority. Can easily leave as is.
The rest is more or less just nagging.
|
|
||
| def _update_project_stats( | ||
| self, | ||
| project_state: ModeStats, |
There was a problem hiding this comment.
NAG[Proj+Aggr]: Same here, perhaps a list then?
There was a problem hiding this comment.
Done, as a list.
|
NOTE: |
c3a2dca to
054374f
Compare
mart-r
left a comment
There was a problem hiding this comment.
Looking great!
Thanks for the work!
Hihi,
A new world for get_stats.
Adds three new character metrics:
Adds two new "modes":
Minor changes:
I'm a bit unhappy with the naming of the pydantic structure of "RawStats", "ProjectStats", "ModeStats"... It kind of makes sense but is a bit sloppy when reusing it.