5 ms·
VidGear: Video processing library for Python lovers
- y4mi 7y agoHaha, that example code made my day # check if frame is None if frame_stab is None: #if True break the infinite loop break A single comment saying "continue if stream ends" would be much clearer
- jhack 7y agoOr no comment at all. This code is as self-documenting as you can get.
- BiteCode_dev 7y ago"# check if frame is None" is indeed a good example of comments you should not write. It's weird that someone has the skill to create what seems such a complete lib and yet produces what most would qualify as a beginner structure. If the project holds up, it's definitely a teaching moment for me.
- ben509 7y agoI noticed reading some of the comments here that they don't get into what a "good" comment is. Many critiques on HN will try to identify best practices, but with comments, and writing generally, it's more a skill you have to learn. The weirdness you mention can be explained, I think, by the fact that the project has a single contributor.[1] A person learns which patterns work or not when patterns clearly work or fail. Since bad comments are something that affects maintenance without breaking the code, it wouldn't be surprising if the internals are solid while the code is less maintainable. And I predict that if it takes off, you'd see other contributors pushing back against the extraneous comments, and Abhishek's style in commenting evolve. [1]: https://github.com/abhiTronix/vidgear/graphs/contributors https://github.com/abhiTronix/vidgear/graphs/contributors
- dr_zoidberg 7y agoI really dislike how OpenCV handles reading frames from a video file, leaving up to the developer the responsibility of checking if it's None or not (or the status) -- the whole thing feels wrong in Python. It's actually super simple to write a wrapper around it that lets you do: for frame in video: # your processing comes here and you forget # having to check for None, while True s and things # that should've stayed in the stone age And I'm a little disappointed the posted library doesn't do it.
- compressedgas 7y ago`iter` can be used to convert from a repeated function call with a sentinel to an iterator: for frame_stab, frame_org in zip(iter(stream_stab.read, None), iter(stream_org.read, None)): ... This line replaces the first 12 lines of the loop in the example.
- BiteCode_dev 7y agoAlso a good example where 3.8 would be handy: while (frame_stab := stream_stab.read()) and (stream_org := stream_org.read()): iter() works with all stable versions of Python though, but it's a bit exotic. When I use it, I tend to make aliases to make it clearer because most Python devs don't know about it, let alone this alternative form. stab_frames = iter(stream_stab.read, None) org_frames = iter(stream_org.read, None) for frame_stab, frame_org in zip(stab_frames, org_frames):
- abhiTronix 7y agostab_frames = iter(stream_stab.read, None) org_frames = iter(stream_org.read, None) for frame_stab, frame_org in zip(stab_frames, org_frames): wow, I did'nt knew about this. Thanks for posting. I'm definetly adding this in the next release.
- abhiTronix 7y agoThanks for suggesting this, I'm always open to good suggestions. The reason behind comments like this is that I thought it would a good thing to explain the code as a newbie's perspective, after all, my primary goal was to make this library approachable to all audiences, especially someone who doesn't know how to program very well. Being said that I'll soon fix these redundant comments.
- mathgenius 7y agoPretty much every buzzword used halves my expectation that this code will actually work... But I hope it does because this actually looks useful.
- batt4good 7y agoThis is awesome and I really hope it works. I still wonder how well it actually performs though (assuming it actually works).
- abhiTronix 7y agoYup, It is all working, well-tested and under continuous development. If still, something doesn't work for you then report an issue[1]. Goodluck. [1]: https://github.com/abhiTronix/vidgear/issues/new/choose https://github.com/abhiTronix/vidgear/issues/new/choose