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

format_detection: add a dedicated function to report unsupported detection #665

Merged
merged 1 commit into from
Feb 15, 2022
Merged

format_detection: add a dedicated function to report unsupported detection #665

merged 1 commit into from
Feb 15, 2022

Conversation

IRDonch
Copy link

@IRDonch IRDonch commented Feb 14, 2022

Summary

Currently, instances where detection for a format is unsupported are reported via context.fail. However, context.fail is intended primarily for situations where the dataset fails some requirement, and using it for this leads to unnatural-looking messages:

The following requirement was not met:
  this format cannot be autodetected

Fix this by adding a dedicated rejection reason for this case and a new method, context.raise_unsupported to abort detection for this reason.

How to test

Checklist

License

  • I submit my code changes under the same MIT License that covers the project.
    Feel free to contact the maintainers if that's a concern.
  • I have updated the license header for each file (see an example below)
# Copyright (C) 2021 Intel Corporation
#
# SPDX-License-Identifier: MIT

…ction

Currently, instances where detection for a format is unsupported are
reported via `context.fail`. However, `context.fail` is intended primarily
for situations where the dataset fails some requirement, and using it for
this leads to unnatural-looking messages:

    The following requirement was not met:
      this format cannot be autodetected

Fix this by adding a dedicated rejection reason for this case and a new
method, `context.raise_unsupported` to abort detection for this reason.
Comment on lines +208 to +209
def detect(context):
context.raise_unsupported()
Copy link
Contributor

@zhiltsov-max zhiltsov-max Feb 14, 2022

Choose a reason for hiding this comment

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

It looks like it should be the default implementation (in Importer or Environment), shouldn't it?

Copy link
Author

Choose a reason for hiding this comment

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

I considered this, but I think the current default implementation (falling back to find_sources) is more useful.

Copy link
Contributor

Choose a reason for hiding this comment

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

When it's supposed to be called then?

Copy link
Author

Choose a reason for hiding this comment

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

I have added calls to it in this PR.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ok, I expect it to be redesigned quite soon though.

@zhiltsov-max zhiltsov-max merged commit f7ee56b into openvinotoolkit:develop Feb 15, 2022
@IRDonch IRDonch deleted the detection-unsupported branch September 9, 2022 22:54
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