Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Proposal]: Please only print banner if I call/use vidgear, not at import #338

Closed
4 tasks done
hughperkins opened this issue Nov 6, 2022 · 5 comments
Closed
4 tasks done
Assignees
Labels
ENHANCEMENT ⚡ New Feature/Addition/Improvement PROPOSAL 📩 A proposal/proposition SOLVED 🏁 This issue/PR is resolved now. Goal Achieved!

Comments

@hughperkins
Copy link

Issue guidelines

Issue Checklist

  • I have searched open or closed issues and found nothing related to my idea.
  • I have read the Documentation and it doesn't mention anything about my idea.
  • To my best knowledge, my idea wouldn't break something for other users.

Describe your Idea

Please only print banner if I call/use vidgear, not at import.

Currently I've moved the import to below an if guard

    if our_args.record:
        import vidgear.gears

... but lots of linters don't like this kind of construction.

I'm happy to have a banner showing, but would prefer it only to show when I'm actually using/calling the library, not simply at import, please.

Note that I have not read the issue guidelines, nor searched open and closed issues etc. I just ticked the boxes because it was mandated by this form. Please feel free to close this issue on that basis.

Use Cases

Means I don't need to add a guard around the vidgear import, in order not to get a banner when I'm not using it.

Any other Relevant Information?

No response

@hughperkins hughperkins added the PROPOSAL 📩 A proposal/proposition label Nov 6, 2022
@welcome
Copy link

welcome bot commented Nov 6, 2022

Thanks for opening this issue, a maintainer will get back to you shortly!

In the meantime:

  • Read our Issue Guidelines, and update your issue accordingly. Please note that your issue will be fixed much faster if you spend about half an hour preparing it, including the exact reproduction steps and a demo.
  • Go comprehensively through our dedicated FAQ & Troubleshooting section.
  • For any quick questions and typos, please refrain from opening an issue, as you can reach us on Gitter community channel.

@abhiTronix
Copy link
Owner

@hughperkins I think this issue is very thought upon when creating this banner, it was necessary because some user never mention vidgear version when filing for issue here.

Currently I've moved the import to below an if guard

if our_args.record:
    import vidgear.gears

... but lots of linters don't like this kind of construction.

Yes this is bad way to tackle it. I'll think other way to display it only once the Class/API is called. I'll notify you here when it is done.

@abhiTronix abhiTronix added ENHANCEMENT ⚡ New Feature/Addition/Improvement WORK IN PROGRESS 🚧 currently been worked on. labels Nov 17, 2022
@abhiTronix abhiTronix self-assigned this Nov 17, 2022
@hughperkins
Copy link
Author

Thank you!

abhiTronix added a commit that referenced this issue Jan 4, 2023
…lled, not at import (Fixes #338)

- ✨ Added `logcurr_vidgear_ver` helper function to facilitate logging current vidgear version, when called within a API.
- 🔇 Implemented `ver_is_logged` global variable to log version only once, modifiable only with `logcurr_vidgear_ver`, and finally made it available in all APIs.
- 🧑‍💻 Followed recommendation given in official python docs: https://docs.python.org/3/faq/programming.html#how-do-i-share-global-variables-across-modules
- ⚡️ Current vidgear version can only be logged by APIs with its logging enabled.
- 🔥 Removed unnecessary imports.
@abhiTronix
Copy link
Owner

abhiTronix commented Jan 4, 2023

@hughperkins With PR #348, you can now completely turn off logging and moreover APIs with logging turned on (logging=True) will automatically log version only once when called within code, thus serving both purposes. Kindly test it if you could by cloning and installing vidgear's development branch as follows:

# clone the repository and get inside
git clone https://github.com/abhiTronix/vidgear.git && cd vidgear

# checkout the latest testing branch
git checkout development

# Install latest stable release with all Core dependencies
pip install .[core]

Then test if problem still presists. Goodluck!

@abhiTronix abhiTronix added WAITING TO TEST ⏲️ Asked user to test the suggested example/binary/solution SOLVED 🏁 This issue/PR is resolved now. Goal Achieved! and removed WORK IN PROGRESS 🚧 currently been worked on. labels Jan 4, 2023
@hughperkins
Copy link
Author

Super! thank you :)

@abhiTronix abhiTronix removed the WAITING TO TEST ⏲️ Asked user to test the suggested example/binary/solution label Jan 4, 2023
abhiTronix added a commit that referenced this issue Jan 4, 2023
…lled, not at import. (Fixes #338) [#348]

- ✨ Added `logcurr_vidgear_ver` helper function to facilitate logging current vidgear version, when called within a API.
- 🔇 Implemented `ver_is_logged` global variable to log version only once, modifiable only with `logcurr_vidgear_ver`, and finally made it available in all APIs.
- 🧑‍💻 Followed recommendation given in official python docs: https://docs.python.org/3/faq/programming.html#how-do-i-share-global-variables-across-modules
- ⚡️ Current vidgear version can only be logged by APIs with its logging enabled.
- 🔥 Removed unnecessary imports.

🚑️ CI: Fixed missing v4l2loopback apt dependency on Linux envs.

🚑️ WriteGear: Fixed gstpipeline_mode not activating when wrongly assuming `output` value as valid path.
    - 👷 Added v4l2loopback support for testing `/dev/video0` device on Linux machines.
    - ☂️ Increased coverage.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ENHANCEMENT ⚡ New Feature/Addition/Improvement PROPOSAL 📩 A proposal/proposition SOLVED 🏁 This issue/PR is resolved now. Goal Achieved!
Projects
None yet
Development

No branches or pull requests

2 participants