pyetrade icon indicating copy to clipboard operation
pyetrade copied to clipboard

Kwargs expansion, linting, and formatting

Open Robert-Zacchigna opened this issue 1 year ago • 4 comments

Hello again, so it seems I was finally motivated enough to expand and lint kwargs for the etrade api.

THIS WILL BE A BREAKING CHANGE

I expanded all methods using kwargs (except in order.py, that will have to be done another day...) with the api variables and their respective defaults.

Since before you had to use the specific name of the api variable to be expanded by kwargs, the variable names had to be slightly changed to fit python formatting standards.

EXAMPLES:

  • sortBy becomes sort_by
  • startDate becomes start_date
  • sortOrder becomes sort_order
  • etc...

So anyone using kwargs before this point will need to update their variable names to match the ones now added to the methods.

I also applied some spelling, grammar and other general formatting fixes.

@jessecooper @1rocketdude Please review and let me know if anything needs to be changed/fixed. This should hopefully make understanding and using the various methods much easier for people.

Robert-Zacchigna avatar Jan 06 '24 23:01 Robert-Zacchigna

Codecov Report

Attention: Patch coverage is 95.00000% with 2 lines in your changes are missing coverage. Please review.

Project coverage is 90.22%. Comparing base (17a8957) to head (21050b4).

Files Patch % Lines
pyetrade/accounts.py 90.90% 1 Missing :warning:
pyetrade/order.py 75.00% 1 Missing :warning:

:exclamation: Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@            Coverage Diff             @@
##           master      #89      +/-   ##
==========================================
+ Coverage   87.28%   90.22%   +2.93%     
==========================================
  Files           6        6              
  Lines         401      409       +8     
==========================================
+ Hits          350      369      +19     
+ Misses         51       40      -11     

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

codecov-commenter avatar Jan 06 '24 23:01 codecov-commenter

@Robert-Zacchigna Thanks for the PR and apologies for the lack of response. Give me some time to review and test these changes.

jessecooper avatar Jan 19 '24 17:01 jessecooper

@jessecooper just checking in, any issues and/or changes you think i need to make?

Robert-Zacchigna avatar Feb 24 '24 23:02 Robert-Zacchigna

I apologize I have not had a moment to give this a full review. It is next on my backlog. I have not forgotten I promise.

jessecooper avatar Feb 25 '24 00:02 jessecooper

THIS WILL BE A BREAKING CHANGE

I would bump version to at least 1.5 then

csinko avatar Mar 02 '24 21:03 csinko

@Robert-Zacchigna This is a great PR. Thank you for taking your time to make these much needed changes and also cleaning things up and added type annotations. It is very appreciated. Please see comment: https://github.com/jessecooper/pyetrade/pull/89/files#r1526233328. Either you can just change the file back and I can handle the bump after the merge or you can do the bump either way will work for me.

jessecooper avatar Mar 15 '24 12:03 jessecooper

@Robert-Zacchigna This is a great PR. Thank you for taking your time to make these much needed changes and also cleaning things up and added type annotations. It is very appreciated. Please see comment: https://github.com/jessecooper/pyetrade/pull/89/files#r1526233328. Either you can just change the file back and I can handle the bump after the merge or you can do the bump either way will work for me.

No problem at all, I'm glad you found the PR to be of good quality.

I have reverted my version bump, you are free to handle that as you see fit.

I'm sure I'll mosey on back to this again to finish order.py when i have more free time again (if you don't beat me to do it).

Robert-Zacchigna avatar Mar 19 '24 04:03 Robert-Zacchigna

This is released in pyetrade 2.0.1

jessecooper avatar Mar 25 '24 12:03 jessecooper