Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Tweaks, Types, and Cleanup #302

Merged
merged 5 commits into from
Jan 24, 2021
Merged

Conversation

bfcarpio
Copy link
Collaborator

@bfcarpio bfcarpio commented Jan 24, 2021

Mixed bag of changes:

  • Types: I was using this package in a side project and realized that I didn't know what types some of the public functions were retuning intuitively. I ended up looking at the test cases. Types and hints can show up in language servers and pop up docs making it easier to use functions you haven't memorized. I don't intend to add mandatory mypy checking, but I thought this would be useful. Sphinx docs for public functions might also be good for this.
  • Language grabbing: had some linter errors and iterating just to break on the first item seemed a bit clunky.
  • Limiting splitting: We use split() in a couple places, but then only grab the 0th, 1st, or -1th item. Doesn't make sense to check the whole string or start from the left in some cases. Tweaked this.
  • Tweaks: isinstance instead of type() == in some places and cleaning some code that seemed repetitive.

Feel free to critique. I'm happy to undo some changes if you feel I got too fancy etc.

@hhursev
Copy link
Owner

hhursev commented Jan 24, 2021

I like everything I see. Thanks to you TIL you can pass connect + read timeout to requests (with a tuple). If no tuple, the float value is applied to both. 🧑‍🎓 📚 I've been using the requests lib for a decade and didn't know this. Gives me some food for thought on typing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants