search

Lightmatter / welkin-health

A Python wrapper of the Welkin Health API
https://welkin.readthedocs.io
Other
4 stars 3 forks source link

WIP feature/paging #23

Closed alonzopt closed 2 years ago

alonzopt commented 2 years ago

Still a little messy/light on comments but passes read-all tests for users, patients, and calendar events.

Had to move base PageIterator class above Collection in models/base.py to make vscode respect the reference.

PageableIterator and MetaInfoIterator are for existing the paging types, MetaIterator is for chat. Class names are from their key in the response, very willing to change them.

Will need to rebase for CDT stuff, but at a glance the 4th paging style should be pretty easy to make a Iterator class for.

alonzopt commented 2 years ago

What's the purpose behind the getter/setter methods here? Also, if we do need to use getters/setter methods, I'd suggest using this pattern:

@property
def foo(self):
    return self._foo

@foo.setter
def foo(self, value):
    self._foo = value

_set_last can be changed to a setter since that's going to be universal (at least for the 4 types we have now)._set_page is a little more complicated since the token based pagination (MetaIterator) doesn't use pages so that needed to be broken out. It might just be easier to merge the functions into something named like _update_self_after_request(meta) and the subclasses can do whatever they need in that.

The get methods can probably just be moved to variables, I had started with the sets and was probably overthinking the design and trying to keep that stuff hidden when it doesn't need to be.

codecov[bot] commented 2 years ago

Codecov Report

Merging #23 (7763763) into develop (119127d) will decrease coverage by 0.39%. The diff coverage is 89.32%.

@@             Coverage Diff             @@
##           develop      #23      +/-   ##
===========================================
- Coverage    92.29%   91.90%   -0.40%     
===========================================
  Files           20       21       +1     
  Lines          818      877      +59     
===========================================
+ Hits           755      806      +51     
- Misses          63       71       +8
Impacted Files Coverage Δ
welkin/pagination.py 86.74% <86.74%> (ø)
welkin/client.py 90.62% <100.00%> (+0.19%) :arrow_up:
welkin/models/base.py 77.38% <100.00%> (-4.44%) :arrow_down:
welkin/models/calendar.py 93.75% <100.00%> (+0.41%) :arrow_up:
welkin/models/cdt.py 94.44% <100.00%> (+0.32%) :arrow_up:
welkin/models/encounter.py 93.02% <100.00%> (+0.34%) :arrow_up:
welkin/models/patient.py 95.23% <100.00%> (+0.50%) :arrow_up:
welkin/models/user.py 80.00% <100.00%> (+1.21%) :arrow_up:

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

alonzopt commented 2 years ago

Code coverage dropping is due to the inclusion of the pagination class for chat. Did not want to comment out a class for something that will be in use soon.

alonzopt commented 2 years ago

All suggestions were good and have been implemented.

Reminder the test coverage drops because chat pagination class is included in the pagination file. Test coverage passes in chat PR https://github.com/Lightmatter/welkin-health/pull/26