kbr / fritzconnection

Python-Tool to communicate with the AVM Fritz!Box by the TR-064 protocol and the AHA-HTTP-Interface
MIT License
304 stars 59 forks source link

Move exceptions out of top package level? #17

Closed Themanwithoutaplan closed 4 years ago

Themanwithoutaplan commented 4 years ago

The exceptions are are in the top-level namespace of the package. This obviously for a reason but I find it unusual and sightly confusing.

kbr commented 4 years ago

The idea was to import the Exceptions from fritzconnection.core.exceptionsto top level so that users of the package can address exceptions like from fritzconnection import FritzServiceError etc.

Themanwithoutaplan commented 4 years ago

So basically this is about masking the core namespace? The top-level namespace should contain the things users need directly.

I'd suggest simply fritzconnection.exceptions as the proper place for them and possibly an __all__ definition for a "safe" from fritzconnection.exceptions import *

kbr commented 4 years ago

It's not about masking, it's just for convenience. fritzconnection.exceptions would be also ok for me. An __all__ definition may be a good idea in case someone uses a * import (which I really discourage).

Themanwithoutaplan commented 4 years ago

I think we should be explicit here. I'm not sure that client generally wants the exceptions and, if your editor does autocompletion, getting FritzConnection is a challenge? ;-)

import * is there because it's useful.

kbr commented 4 years ago

„Usefulness“ does not prevent it from being disliked by me ;-)

Themanwithoutaplan commented 4 years ago

„Usefulness“ does not prevent it from being disliked by me ;-)

Of course not, which is why I suggested __all__. I'll see if I can do this week. NB. docs look like they might need updating.

kbr commented 4 years ago

Yes, docs represent the 0.x versions.

kbr commented 4 years ago

Removing most of the shortcut imports it looks better now. Also not harder to use. 993da135e857dc147bb9db578f6a74d3778da8ab An __all__ is also included now e7cc5456412ca8b6dcb482f91876760c302b8104

kbr commented 4 years ago

docs are also updated now. So I like to close this.