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.

Sign up for GitHub

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

Jump to bottom

API Suggestions #10

Open
NickCrews opened this issue Mar 13, 2022 · 3 comments
Open

API Suggestions #10

NickCrews opened this issue Mar 13, 2022 · 3 comments

Comments

@NickCrews
Copy link

Hi! Thank you for this library, it has been quite useful to me. As I've used it I've come across a couple of usage pain points and have a few ideas on how it could be improved. Some of the suggestions would be breaking for users, which is a bummer. Not sure how acceptable that would be for you. I could implement them, if you would be willing to merge my PRs. Thanks for your consideration.

Unify error behavior between single/multiple requests.

Sometimes I make requests with one address, sometimes with multiple. It is annoying that I need two different code paths to deal with the errors when an address isn't found, one where the error is thrown (single address) and one where it is just returned (multiple).

I think all errors should just be returned, and never thrown. It makes things consistent, and isn't any more code for users (in fact it may be less to check result as opposed to filtering between the various errors that could get thrown). This of course would break some users.

Don't squash info in errors

Currently, the detailed error info is just string formatted into the ValueError when an address isn't found, so it's difficult for users to recover the original error. The original info should be included somehow. This wouldn't need to be breaking.
@thelinuxkid
Copy link
Owner

Hi Nick. Sorry for the late reply. Thanks for the feedback on this. I think if we document this well we can introduce the breaking changes as a new version. This library has been out for a few years and it's natural that it should evolve. Please send the PR over and I'll review.

Thanks.

@NickCrews
Copy link
Author

Thanks Andrés. I'll do that! I notice that this is still compatible with python2, can I get rid of support for that? It will allow me add some typing and some other nice new language constructs.

@thelinuxkid
Copy link
Owner

That will work! Thanks Nick.

NickCrews added a commit to NickCrews/pyusps that referenced this issue Apr 5, 2022
@NickCrews
Accept Iterable, return list, return errors
6bf0575 
See thelinuxkid#10

Switching away from *args makes it so that
single inputs are handled consistently.
Otherwise, users won't know what sort of response
to expect from passing a single input:
Will they get a single item back, or a list
of length one?
Just make it so you always pass a list.

Or, in fact, make it so that this accepts any iterable, not just a list
or some other iterable with a __len__ method.
NickCrews added a commit to NickCrews/pyusps that referenced this issue Apr 5, 2022
@NickCrews
Return USPSError, not ValueError
f62a593 
See thelinuxkid#10

- still allows you to catch ValueErrors
- still acts just like a ValueError
- Differentiates between errors from USPS,
  and the ValueError you get if you supply more than 5 addresses
NickCrews added a commit to NickCrews/pyusps that referenced this issue Apr 6, 2022
@NickCrews
Accept Iterable, return list, return errors
21d8dd1 
See thelinuxkid#10

Switching away from *args makes it so that
single inputs are handled consistently.
Otherwise, users won't know what sort of response
to expect from passing a single input:
Will they get a single item back, or a list
of length one?
Just make it so you always pass a list.

Or, in fact, make it so that this accepts any iterable, not just a list
or some other iterable with a __len__ method.
NickCrews added a commit to NickCrews/pyusps that referenced this issue Apr 6, 2022
@NickCrews
Return USPSError, not ValueError
60fa7f1 
See thelinuxkid#10

- still allows you to catch ValueErrors
- still acts just like a ValueError
- Differentiates between errors from USPS,
  and the ValueError you get if you supply more than 5 addresses
NickCrews added a commit to NickCrews/pyusps that referenced this issue Apr 6, 2022
@NickCrews
Accept Iterable, return list, return errors
ec9ade3 
See thelinuxkid#10

Switching away from *args makes it so that
single inputs are handled consistently.
Otherwise, users won't know what sort of response
to expect from passing a single input:
Will they get a single item back, or a list
of length one?
Just make it so you always pass a list.

Or, in fact, make it so that this accepts any iterable, not just a list
or some other iterable with a __len__ method.
NickCrews added a commit to NickCrews/pyusps that referenced this issue Apr 6, 2022
@NickCrews
Return USPSError, not ValueError
3f28f07 
See thelinuxkid#10

- still allows you to catch ValueErrors
- still acts just like a ValueError
- Differentiates between errors from USPS,
  and the ValueError you get if you supply more than 5 addresses
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@thelinuxkid @NickCrews