search

jeffbass / imagezmq

A set of Python classes that transport OpenCV images from one computer to another using PyZMQ messaging.
MIT License
1.01k stars 160 forks source link

v0.1.1-beta1 - Some Improvements #28

Closed StiiCeva closed 4 years ago

StiiCeva commented 4 years ago

I plan on adding new improvements and maybe implement a "Paranoid Pirate" recipe for a multi video source to multi-worker usecase.

Cheers, Alex

PS: My first pull request!!! :D

jeffbass commented 4 years ago

Hi Alex, Thanks for your pull request. I'm going to need a couple of weeks to read it and get back to you. Thanks for your patience! Jeff

StiiCeva commented 4 years ago

Hi Jeff,

Just noticed you got to version 0.3.0 (on master) with a pull request from someone. Most of the modifications are compatible with the current version of the master branch.

There is not hurry.

Have a great day, Alex

On Thu, Dec 12, 2019 at 4:46 AM Jeff Bass notifications@github.com wrote:

Hi Alex, Thanks for your pull request. I'm going to need a couple of weeks to read it and get back to you. Thanks for your patience! Jeff

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/jeffbass/imagezmq/pull/28?email_source=notifications&email_token=ABMBSSGX67SD4QNVINME4KLQYGQZNA5CNFSM4JZU4QQKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEGVI4HY#issuecomment-564825631, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABMBSSEFUX3QOFV7VZKZYQTQYGQZNANCNFSM4JZU4QQA .

jeffbass commented 4 years ago

The recent 0.3.0 modifications were from a pull request that added "pub / sub" pattern capability. You can see that set of changes in the "add-pub-sub" branch. I pulled those changes into master (but kept the branch in the repository for now as a reference). I am still making changes to the documentation files to fully "catch them up" to the addition of the pub / sub capability. For the next version number increment, I am making a lot of documentation improvements and changing the organization of the current documentation files.

jeffbass commented 4 years ago

Hi Alex, I've been looking at your pull request and have a few comments. You mentioned this is your first pull request -- congratulations on that! Here are a few suggestions that help pull requests and code modifications to go more smoothly.

  1. Before submitting a pull request, do a fresh pull from the source repository (this one) to your forked repository. Merge and resolve any changes. Then make your pull request. That way, you making changes to the most current version of the code in the source repository.
  2. Please do a separate pull request for each different change you are suggesting. Each suggested code change should be single new piece of functionality or fix a specific bug. Each pull request should be named in a way that makes it clear what is being added or changed in that pull request. For example, your pull request "v0.1.1-beta1 - Some Improvements" contains 4 different ideas in a single pull request and that makes managing changes and doing testing difficult. It would be helpful to submit a separate pull request for each separate idea:
  3. Make changes in .gitignore (yes it is simple, but it needs its own separate pull request)
  4. Make imagezmq pip installable (see below for my comment on pip install)
  5. Add ReliableImageSender class (this is a significant new class--thanks!)
  6. Add debug code to imagezmq.py (see my comment below)

It would be helpful if each of these were a separate pull request. I can merge the first one (changes to .gitignore), but the others will each need their own discussion, back and forth comments and modifications.
Also, I am very reluctant to add any if statements or print statements (for debugging or any other reason) to the imagezmq classes. These classes are optimized for speed in my own production systems, so I won't be incorporating those changes. I do all debugging in the test programs and application programs that use the imagezmq classes, not in the classes themselves. Speed is very important for my applications (see my imagenode and imagehub repositories). I had already started the code for making imagezmq pip installable using a very different template than the one you suggested. I'll start a branch for that and you can help improve it. I think your ReliableImageSender Class is an interesting idea that I look forward to working with you on. It would benefit from documentation; I suggest putting that documentation in a new file in the docs folder. Thanks for your ideas and code.

Next steps.

  1. I am going to close this pull request.
  2. Please do a fresh pull of this repository so your next pull requests use this as a base.
  3. Merge your .gitignore changes and do a new pull request specifically for that
  4. I'll merge that pull request (and you will have completed your first merged pull request and be listed as a contributor to the imagezmq master branch)
  5. Make a new pull request for adding your ReliableImageSender class. I would suggest starting a new git branch for that. We will then work through developing it, especially the documentation. I think that it will be an interesting addition to the repository.
  6. I will be adding my own version of pip install code, rather than using the template in your pull request. I will ask for your feedback and suggestions in a new pull request and incorporate them. You'll be listed as a contributor.
  7. As we incorporate your new classes, I will assign version numbers; no need to change version numbers in a pull request. Thanks for your help improving imagezmq. I look forward to working with you on your suggested classes. Jeff
jeffbass commented 4 years ago

I am closing this pull request so you can submit separate pull requests per my comments above.

StiiCeva commented 4 years ago

Sorry for the late reply @jeffbass .

I'll do as instructed and come back to you.

jeffbass commented 4 years ago

No worries! imagezmq is not the day job for any of us ;-) As you can see, imagezmq has had a major release to version 1.0.1 after multiple years of testing. Setup.py is tested and done. imagezmq is now pip installable and set at a stable API. I also uploaded it to PyPI, so it is pip installable for anyone. Looking forward to working with you on a ReliableImageSender class in a new branch.