Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

type hint changes in dictlist.py #1256

Open
wants to merge 2 commits into
base: devel
Choose a base branch
from

Conversation

akaviaLab
Copy link
Contributor

  • fix #(issue number)
  • description of feature/fix
    Changed type hints in dictlist to be TypeVar(Object), because otherwise my linter would complain about expecting a Reaction and getting Object from model.reactions.get_by_id(). Similarly for other interactions with dictlists.
    Doesn't change functionality.
  • tests added/passed
  • add an entry to the next release

@codecov-commenter
Copy link

codecov-commenter commented Jul 26, 2022

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 84.41%. Comparing base (2d343e7) to head (b33ad79).
Report is 93 commits behind head on devel.

Additional details and impacted files
@@            Coverage Diff             @@
##            devel    #1256      +/-   ##
==========================================
+ Coverage   84.31%   84.41%   +0.09%     
==========================================
  Files          66       66              
  Lines        5497     5498       +1     
  Branches     1265     1265              
==========================================
+ Hits         4635     4641       +6     
+ Misses        556      551       -5     
  Partials      306      306              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@akaviaLab
Copy link
Contributor Author

@cdiener - can you please review?

@cdiener
Copy link
Member

cdiener commented Aug 18, 2022

Not an expert in type annotations. Looks a bit hacky to me. Could it not just by the Any type for the items in a DictList since it's a general purpose container?

@cdiener cdiener added the stale The issue or pull request lacks activity. label Apr 8, 2024
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
stale The issue or pull request lacks activity.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants