ahupp / python-magic

A python wrapper for libmagic
Other
2.59k stars 280 forks source link

Expose all arguments in `from_*` methods #304

Closed maringuu closed 10 months ago

maringuu commented 10 months ago

As a user I don't really care about the Magic class. All I want is to query the magic of some files. Before this commit you had to create an instance of Magic. After this commit you can pass all arguments you would pass to its constructor to the instanceless methods.

I am pretty happy with this API, are you?

ahupp commented 10 months ago

From the perspective of a new user of the library, the magic.from_* has a very simple signature that's immediately clear how to use. The magic.Magic class takes a large number of arguments (some obscure) and is intended for cases like yours where you might want customization. So the split here is intentional.

Is it that arduous to create a new instance?

m = magic.Magic(<some arguments)
m.from_file(...)

vs

magic.from_file(..., <some arguments>)

Separate from that, **kwargs on python functions is a usability pain because it's not clear what args it takes.

maringuu commented 10 months ago

Is it that arduous to create a new instance?

Well kind of. I just don't want to keep track of the instances. So I'll create an instance for every call I make, which I don't like. I figured since there already is some logic to keep track of instances this change would be a good fit.

the magic.from_* has a very simple signature that's immediately clear how to use.

I don't see how they are more intuitive in status quo. You have to read the docstring to know what it does (I agree that it is kind of obvious for the mime argument). With this change you would read the docstring then head over to Magic and see what you want there.

large number of arguments (some obscure)

I would argue that you should not hide these arguments for the sake of simplicity. They already have defaults so the user can just rely on them. What is the downside of pointing the user to the full docs?