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

Added Inference IDs to Sources when Uploaded via the Dataset Upload Block #571

Closed
wants to merge 4 commits into from

Conversation

chandlersupple
Copy link
Contributor

Description

When an image is uploaded through Active Learning, Model Monitoring allows users to view the image in its "Recent Inferences" table. This currently works with Active Learning, however, it does not work with the new "Roboflow Dataset Upload" block in Workflows.

Modified the Dataset Upload block to provide the inference_id to register_image_at_roboflow() (As is currently being done in Active Learning).

Type of change

  • Bug fix (non-breaking change which fixes an issue)

How has this change been tested, please provide a testcase or example of how you tested the change?

  • Passed all unit and integration tests.
  • Confirmed that images show up in Model Monitoring with the changes in this PR.

Any specific deployment considerations

N/A

Docs

N/A

@chandlersupple chandlersupple marked this pull request as ready for review August 8, 2024 00:34
@@ -457,13 +457,19 @@ def register_datapoint(
batch_name: str,
tags: List[str],
) -> str:
inference_id = (
Copy link
Collaborator

Choose a reason for hiding this comment

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

Could u please put todo comment linking this issue: #567

Copy link
Collaborator

@PawelPeczek-Roboflow PawelPeczek-Roboflow left a comment

Choose a reason for hiding this comment

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

Blocked by: #565

Copy link
Collaborator

@PawelPeczek-Roboflow PawelPeczek-Roboflow left a comment

Choose a reason for hiding this comment

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

There is bug in this submission - does not work with classification
also revealing weakness of the tests

@PawelPeczek-Roboflow
Copy link
Collaborator

Updated my branch with your changes fixing issues I spotted and adding tests: 0962acd

@chandlersupple please verify + suggestion for next contribution - if you change behaviour of code such that you add / remove parameter and this is somewhat important change you shall expect some test to fail - if that's not the case and you care about change you should modify existing tests / add new test to ensure coverage

@PawelPeczek-Roboflow
Copy link
Collaborator

to be closed soon

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.

2 participants