ua-parser / uap-python

Python implementation of ua-parser
Apache License 2.0
561 stars 152 forks source link

Does Parser make sense as a super class? #189

Closed masklinn closed 7 months ago

masklinn commented 8 months ago

Need to go back to the original consideration about them: cross-parser delegation can only happen with __call__, the parse* helpers can't be overridden, because each parser layer will immediately call into its own __call__, there is no parse* calling through the chain.

As such, it may not make that much sense for Parser to be an abstract class at the top of the hierarchy? Maybe each parser should just be a callable (with the current __call__ method), and the "parser" interface would be a top-level helper / wrapper around a pile of whatever the callables are (resolver?)

This also means the simpler thingies can be straight up functions e.g. the noop parser from the hitrates script (the counter probably still needs to be a class in order to hold state).

On average it doesn't actually change anything, save removing the inheritance, because almost every parser has state, that state could be reified as a closure instead but I'm not entirely sure it's valuable... though maybe it is? Check out how it works.

edit: trying it out, it does make parsers (resolvers?) a bit simpler to implement, but instantiating one kinda feels bad as you need to wrap the same object around the stack of resolvers every time.

Maybe the wrapper is not necessary though? Instead the top-level utility functions can do the job and take an optional parser? On the other hand that seems a bit dubious from a reliability perspective, it seems way too easy to call ua_parser.parse(s) when you wanted ua_parser.parse(s, myparser). And removing the easy interface or requiring explicitly passing a None parser (or w/e) is pretty awful.

masklinn commented 7 months ago

https://github.com/python/mypy/issues/8235#issuecomment-1843669422 for a way to test protocol conformance relatively easily