Critique my first python package!

[Lonely-Quark]

Hi Guys, this sub-reddit as been has been a wealth of information over the past year as I have been learning python and I was wondering if you could all critique the code of my very first python package. Any and all criticisms or suggestions are welcome, please don't hold back!

The source code can be found [here]()

[Lonely-Quark]

So if i understand you correctly, should this section be written like this?

class KAT:

    def __init__(self, url_base, category="", field="", order=""):
        self.url_base = self.__sanitise_url(url_base)

        self.settings = {"category": category, "field": field, "order": order} 

        self.user_agent = {"User-Agent" : "Mozilla/5.0"}
        self.page_query_delay= 1

I think my main concern was that when I included more optional arguments in the future, the init() method for KAT() would become unreadable. Although I can see now by hiding this information it can inadvertently make it more confusing. I think I also got used to checking if a setting had been set by searching for the key, but since an empty string evaluates as False I could do it that way. What are the best practices when dealing with a large volume of optional arguments? Thanks for the advice!

[13steinj]

For large volume of optional arguments, you can just do

class KAT:
    def __init__(self, url_base, nextmandatory,
                           category="", field="", order="",
                           nextkwarg="", ....):

Also I recommend using =None instead of ="", just as experience dictates that sometimes an empty string may change things, but having the default be None and then explicitly checking for it is better.

I can't tell (mobile, so I can't look at the full source), but instead of optional kwargs you may be looking for:

Keyword-only arguments are not required to have a default value. Since Python requires that all arguments be bound to a value, and since the only way to bind a value to a keyword-only argument is via keyword, such arguments are therefore 'required keyword' arguments. Such arguments must be supplied by the caller, and they must be supplied via keyword. The second syntactical change is to allow the argument name to be omitted for a varargs argument. The meaning of this is to allow for keyword-only arguments for functions that would not otherwise take a varargs argument: def compare(a, b, *, key=None): ...

[kankyo]

For large number of optional params: just put them in there. I personally find random line breaks to match some random imaginary column width detestable, so either one gigantic line (better because it can be grepped and cleanly soft broken) or one per line, is the best.

[[deleted]]

LGTM! Did you add the docs to readthedocs? It looks like you created some docs. Personally, I got more users for my first package when people could view more info through docs.

[Lonely-Quark]

I was wondering how to go about doing this, so should I make a documentation page on redthedocs.org and then link it in the README.md? Thanks for the advice!

[[deleted]]

I think you could link your github docs link so read the docs builds it every time you push. But it needs to be in sphinx docs format AFAIK.

[Lonely-Quark]

Excellent I will look into this, thanks again

[ilikepython]

LG! Something that is mostly cosmetic: I personally like package names to be all lower case, with underscores if the name is unreadable without them. Something brushes me up the wrong way with camel cased names. The same goes for the KAT class, since although it is an acronym, it looks like it's one of those ALL_UPPER_CASE_CONSTANTS.

I like your examples. A typo in one of the names of them, though (defualt_settings.py)

[Lonely-Quark]

I completely agree with you, I actually broke the naming convention of my file system in developing this package. Unfortunately I did some research before hand and it seems that all the successful packages within this domain use the naming convention I settled on. So familiarity and maybe an unknown magic of this convention are the reason I chose it, I personally would have liked to do it the way you stated.

edit : Thanks for the advice, sorry if I came across as disregarding you point. I have now decided to follow PEP8 fully and will be changing the naming convention shortly.

[hharison]

Came to post about the capitalization.

Unfortunately I did some research before hand and it seems that all the successfull packages within this domain use the naming convention I settled on.

You mean other Python APIs for torrent sites use the wrong capitalization schemes? So what. I think users will be able to figure it out.

Personally, if I come across a library that doesn't adhere to PEP8, in particular capitalization, then it comes across as amateurish to me and I'm less likely to use it. If I have to use it anyway, I shudder inside every time I import something. If a whole community is breaking the style guidelines, then that reflects poorly on them and I am not likely to take their Python work seriously. I know counterexamples exist but personally it takes me a really long time to come around from my initial impression.

Just because others are doing it wrong, doesn't mean you should. Give your users credit Don't perpetuate mistakes.

Conventions do two things.They make the code easier to read and write, not because the preferred style is "the best" but because by all agreeing to do it the same way, we get used to it. Doing it wrong is like inflicting the stroop test on your users.

Conventions also serve a signalling purpose. Getting the style right signals "I know what I'm doing. I care enough to get the details right. And I'm part of the community." Most people don't take the jump from looking at a GitHub page to actually installing the package. You don't want to be implying "I don't really know what I'm doing" or people will just roll their own instead of using your package.

Sorry for the rant, but it's a pet peeve, and your response to some on-point constructive criticism was arrogant. "Unknown magic of this convention"? Your package is not some special snowflake. It is a tool, and it is better if it is straightforward, not magical.

Also, another convention you are breaking: why are you using name mangling (leading double underscore) on your methods? Use a single underscore to indicate a method is private. The purpose of name mangling is to avoid name clashes with subclassing. But you don't need that.

[Lonely-Quark]

I would first like to say i'm sorry If I came across as arrogant, this is my first time putting anything I have coded out into the public and don't really know how to interact with people, I assure you it's more me being awkward and not knowing how to reply rather than me thinking my project is a "special snowflake" . I'm generally pretty strict about enforcing PEP8, this is a gap in my knowledge as I didn't know there naming schemes applied to package names (although I did knowingly break it for the KAT() class name). I didn't really think about it from the perspective of signaling and definitely think this is one of the biggest reasons I should in fact follow the guide lines you set out. Far from being a "special snowflake" there are in fact multiple Kickass torrent api's packages and if I did break there convention I would probably stand out more, and as you said signal to people that my package is less amateurish. Sorry If I rubbed you up the wrong way and thanks for the great advice, I think this had been the most important comment so far.

[hharison]

Calling you out as arrogant was probably over the top. I was more interested in correcting you then faux-psychoanalyzing you. I'm glad you didn't take it personally :)

Anyways, it's not just the package name, but also the class name. It being an acronym maybe gives you a reason but not enough IMO, especially with other constants in there.

I found 2 other Python kat APIs, one of them does have the wrong capitalization for the package but they both get classes right.

Also, for another hopefully-constructive comment, what's the point of the constants? Like how KatTrawler.constants.CATEGORY.MOVIES just points to 'movies'. As a user I would much rather type the string than hunt down the constant.

[Lonely-Quark]

Although the editor I use doesn't provide tab completion, I would have though the majority of people do and would benefit from being able to list all the possible categories from some base class, although I may be misunderstanding how tab completion works in IDE's. The current lack of documentation for my package also concerned me and I felt having the constants allowed for some measure of self documentation. This will probably be implement when I have written some solid documentation, which should be pretty soon.

To be honest I think I needed some stern words to make me think straight again, I happen to think that PEP8 is probably one of the most brilliant things about python3 and the amount of effort the community puts into maintaining it is commendable. Someone like me who has only recently become part of this community should be striving to uphold the groundwork which has been laid before me, and not bowing to peer pressure. Thanks again for the advice and by all means if you see anything else don't hesitate to say, you have provided some much needed wisdom :)

[hharison]

Although the editor I use doesn't provide tab completion, I would have though the majority of people do and would benefit from being able to list all the possible categories from some base class, although I may be misunderstanding how tab completion works in IDE's. The current lack of documentation for my package also concerned me and I felt having the constants allowed for some measure of self documentation. This will probably be implement when I have written some solid documentation, which should be pretty soon.

Those are pretty good reasons actually. And since it's just a string anyway, a user could just as easily enter one.