joweich / chat-miner

Parsers and visualizations for chats
MIT License
567 stars 56 forks source link

Add typing #98

Closed KianKhadempour closed 1 year ago

KianKhadempour commented 1 year ago

This isn't 100% typed because the libraries that we depend on aren't 100% typed so I had to guess for some things. This caused me to find issue #97 (which you should check out). Apart from typing this PR:

KianKhadempour commented 1 year ago

Pytest errors are because | for Union was added in 3.10. I will fix that.

KianKhadempour commented 1 year ago

CodeCov patch is just complaining that I formatted the abstract methods "incorrectly" even though they are black defaults because if you do it the way I originally did them (when black complained) they count as covered code.

codecov-commenter commented 1 year ago

Codecov Report

Merging #98 (7ed3e91) into main (b908d2e) will decrease coverage by 0.16%. The diff coverage is 70.00%.

:exclamation: Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the Github App Integration for your organization. Read more.

@@            Coverage Diff             @@
##             main      #98      +/-   ##
==========================================
- Coverage   71.63%   71.48%   -0.16%     
==========================================
  Files           1        1              
  Lines         275      277       +2     
==========================================
+ Hits          197      198       +1     
- Misses         78       79       +1     
Flag Coverage Δ
unittests 71.48% <70.00%> (-0.16%) :arrow_down:

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
chatminer/chatparsers.py 71.48% <70.00%> (-0.16%) :arrow_down:
KianKhadempour commented 1 year ago

Codecov Report

Merging #98 (289728f) into main (f7f34ff) will decrease coverage by 0.26%. The diff coverage is 70.73%.

❗ Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the Github App Integration for your organization. Read more.

@@            Coverage Diff             @@
##             main      #98      +/-   ##
==========================================
- Coverage   71.63%   71.37%   -0.26%     
==========================================
  Files           1        1              
  Lines         275      276       +1     
==========================================
  Hits          197      197              
- Misses         78       79       +1     

Flag Coverage Δ
unittests 71.37% <70.73%> (-0.26%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more. Impacted Files Coverage Δ
chatminer/chatparsers.py 71.37% <70.73%> (-0.26%) ⬇️

This is because I added a few variable types.

joweich commented 1 year ago

@BlueishTint commented and resolved all open discussions. Is there anything more you want to bring into this PR before merging?

KianKhadempour commented 1 year ago

Sorry I was gone for a while with no internet access. I think everything is good here.

joweich commented 1 year ago

@BlueishTint fyi, I will fix #99 and merge the fix into this PR. Then I'll merge your PR 👍🏼