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

remove detectors upload routine #244

Merged

Conversation

denniswittich
Copy link
Contributor

Removes the detectors upload mechanism via internal queue.

This way of uploading images was redundant to the async upload method which is from now on the only way to upload images to the learning loop.

@denniswittich denniswittich added this to the 0.22.0 milestone Jan 9, 2025
Comment on lines 40 to 41
warn('The tags field is deprecated and will be removed in a future version.',
DeprecationWarning, stacklevel=2)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice! I didn't know about warnings.warn. In NiceGUI I created warn_once for this purpose:
https://github.com/zauberzeug/nicegui/blob/3d834e539b4e8f54c96e06b8ee6732bf5960beec/nicegui/helpers.py#L14-L21

But it looks like DeprecationWarnings are hidden by default. Is this ok for us? Or should we simply "forget" to specify the warning type?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Both ways seem problematic... I would prefer to forget the type and ensure that the warning is shown.

Suggested change
warn('The tags field is deprecated and will be removed in a future version.',
DeprecationWarning, stacklevel=2)
warn('The tags field is deprecated and will be removed in a future version.', stacklevel=2)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The docs say that it's only ignored if it's not triggered by code in __main__. I tested it in another project that's using RoSys, it won't show up with DeprecationWarning, but without I get a UserWarning.

@denniswittich
Copy link
Contributor Author

@falkoschindler I will assume you merge this PR whenever you are done with the review

@falkoschindler falkoschindler merged commit 5f19006 into main Jan 20, 2025
5 checks passed
@falkoschindler falkoschindler deleted the remove-detectors-upload-mechanism-via-internal-queue branch January 20, 2025 13:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants