bibanon / BASC-py4chan

Python wrapper for 4chan API. The BA's vastly improved fork of Edgeworth's original.
http://basc-py4chan.readthedocs.org/en/latest/index.html
Do What The F*ck You Want To Public License
55 stars 13 forks source link

So many aliases! #4

Closed DanielOaks closed 8 years ago

DanielOaks commented 9 years ago

Just the general presence of aliases scattered throughout the code, eg:

board = Board
number = num = no = post_number
html_comment = orig_comment
body = text = comment
url = post_url
url = thread_url
semantic_url = semantic_thread_url

I think we can cull most of them, personally. Especially now that the docs are actually getting written up. I'm thinking of culling all of these or changing it so that just one attribute is provided in place of the multitude of aliases.


The board/Board one just seems a bit unnecessary to me. Mostly because classes in Python start with uppercase letters, and especially given that it prevents access to the actual board.py module.


We'll change both post_url and thread_url to just url for both objects. Given the object-oriented nature of the library, we shouldn't need to worry about people getting confused. Eg:

<Thread>.url   vs   <Thread>.thread_url
<Post>.url   vs   <Post>.post_url

The second one seems a bit redundant, and given that it simplifies the interface and makes it more consistent between different objects, it makes sense to just combine them.

semantic_thread_url will be changed to semantic_url in the same vain, no need for the thread_ qualifier since it's an attribute of a Thread object.


Comments are a bit weird. We've got HTML comments, or Raw comments, and then 'cleaned' or plaintext comments. I'm thinking remove the aliases, and just have the following attributes on Post objects:

Doing this (separating them out like this into separate attributes) also stops the user from having to decide whether to 'clean' comments when they first initialize the Board object, which does seem a bit out-of-place being there anyway.


So, I'll probably end up doing that in the next day or two, but I'll float it here first and see what you guys think.

antonizoon commented 9 years ago

Cutting down on redundancy is great. The reason behind it all is that py-4chan has been forked and merged so many times by different people, and had it's functions renamed twice already, that each person left behind their own naming standard...

However, before we start refactoring the code, let's merge all of Anorov's last few commits from 4chan4py. He's joined the BASC-py4chan effort, and his 4chan4py fork has a lot of new features to offer.

This way, we won't have to backtrack after basically rewriting the library from scratch.

DanielOaks commented 9 years ago

That does make sense, yeah. I'll go through and merge those features of Anorov's fork we're interested in, been looking through and there's some good stuff to look at!

Anorov commented 9 years ago

One purpose of these was to retain most backwards compatibility with py4chan while still providing some better names.

I'd personally recommend disregarding backwards compatibility and just keeping only the most sensible of the aliases.

The board / Board thing is a philosophy followed by Flask and others, where functions (or anything that acts sort of like a function) provide the most high level API for users and classes provide everything beyond that, e.g. when you need to override functionality. See also Requests' session vs. Session for the same reasons. Personally I don't care that much about this one and it can be confusing to some people, so keeping only Board is fine.

The post number one is tricky, because personally I find the name 4chan's API uses (no) kind of ugly. I don't know which of the aliases is the best.

For posts, sticking with just comment (and its derivatives) is probably ok, as 4chan uses com. I always found it awkward to write thread.topic.comment though, because it's odd to think of the original post as a comment; that was my initial reason for creating the alias.

I fully agree with changing all the url ones; I think those aliases were purely for backwards compatibility.

DanielOaks commented 9 years ago

Hmm, how about using the name content instead of comment? Could fit more nicely.

For post numbers, I'm thinking just change it to post_id, since that's what makes sense and the vernacular that's already in usage around. The 4chan API does use silly names for quite a few things – I suspect just to reduce load (same lines as the 4cdn changes), so we shouldn't mind them too much.

I was thinking so as well, in terms of backwards-compatibility. As far as I know, only BASC-Archiver is basically the only thing using BASC-py4chan at this point, so we should take this chance to straighten it all out and make it nice now, rather than later.

antonizoon commented 9 years ago

Well, in terms of backward compatibility, our plain old legacy py-4chan fork will still be around, since it uses the only py4chan package name on PyPi. Both Anorov and DanielOaks have commit privileges on it, in case 4chan makes URL changes.

Though Anorov does make a good point with board and Board, since it seems to be standard practice in Requests and most other Python libraries. No harm in leaving that alias alone.

DanielOaks commented 9 years ago

Sure, that makes sense. We'll leave the board = Board alias in there, won't mention it in the docs until I take a look and see whether Flask/etc mention them.

Regarding comment, I'm actually thinking it's probably fine to just stick with comment, rather than renaming it content or anything different. I can't think of anything that could really take the place and describe the textual part of a post better than comment does, and even on 4chan itself it's called a comment, so I'm not too worried about changing it.

antonizoon commented 8 years ago

It seems like the majority of the aliases have been culled, with the exception of board = Board and is_op = is_OP. Good job.