You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
MediaPipeline defines several empty or almost empty "overridable" methods, which return things inconsistent with their overrides. I propose making all of them raise NotImplementedError. Alternatively MediaPipeline should just be made an abstract class and all those methods made abstract methods, but I have no idea if that will break anything (e.g. do all children always override all of those methods?).
Another problem is existing tests, that test specifically that e.g. MediaPipeline.media_downloaded() returns a response, which makes no sense to me (normally media_downloaded() returns a file info dict), so all those need to be changed or removed.
And another problem, indirectly related to this, is that this interface is very poorly documented, most of these functions are not mentioned in the docs at all, so it's not always clear what should they take and return (and the code uses many of them as callbacks in long callback chains so it's not clear even from the code).
The text was updated successfully, but these errors were encountered:
Replace all bodies with raise NotImplementedError().
Remove all tests that test calling these methods, they are not useful for anything even now.
Make sure nothing else broke.
Make the methods abstract, make sure nothing broke.
Ideally get some 3rd-party pipelines and check that they didn't break either (or just check that they override all of these methods, directly or via FilesPipeline/ImagesPipeline, in which case they can't break?)?
MediaPipeline
defines several empty or almost empty "overridable" methods, which return things inconsistent with their overrides. I propose making all of them raiseNotImplementedError
. AlternativelyMediaPipeline
should just be made an abstract class and all those methods made abstract methods, but I have no idea if that will break anything (e.g. do all children always override all of those methods?).Another problem is existing tests, that test specifically that e.g.
MediaPipeline.media_downloaded()
returns a response, which makes no sense to me (normallymedia_downloaded()
returns a file info dict), so all those need to be changed or removed.And another problem, indirectly related to this, is that this interface is very poorly documented, most of these functions are not mentioned in the docs at all, so it's not always clear what should they take and return (and the code uses many of them as callbacks in long callback chains so it's not clear even from the code).
The text was updated successfully, but these errors were encountered: