Skip to content

Conversation

ZachCafego
Copy link
Contributor

@ZachCafego ZachCafego commented Jun 30, 2023

Issues:


This change is Reviewable

@ZachCafego ZachCafego requested a review from jrobble June 30, 2023 20:47
@ZachCafego ZachCafego self-assigned this Jun 30, 2023
@ZachCafego ZachCafego changed the title Added rollup functionality, unittest Add rollup functionality to CLIP Detection component Jun 30, 2023
Copy link
Member

@jrobble jrobble left a comment

Choose a reason for hiding this comment

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

Reviewed 1 of 3 files at r1, 2 of 2 files at r2, all commit messages.
Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @ZachCafego)


python/ClipDetection/clip_component/clip_component.py line 31 at r2 (raw file):

import csv
from pkg_resources import resource_filename
from typing import Iterable, Mapping, TypedDict

VScode says Iterable and TypedDict are not used.


python/ClipDetection/clip_component/clip_component.py line 90 at r2 (raw file):

        self._triton_server_url = None
    
    def get_classifications(self, images, job_properties: Mapping[str, str]) -> mpf.ImageLocation:

This should be -> Iterable[mpf.ImageLocation]:


python/ClipDetection/clip_component/clip_component.py line 95 at r2 (raw file):

        self._check_class_list(kwargs['classification_path'], kwargs['classification_list'])

        self._preprocessor = ImagePreprocessor(kwargs['enable_cropping'])

Do the following in __init__:

self._preprocessor = None

Copy link
Member

@jrobble jrobble left a comment

Choose a reason for hiding this comment

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

Reviewable status: all files reviewed, 4 unresolved discussions (waiting on @ZachCafego)


python/ClipDetection/clip_component/clip_component.py line 69 at r2 (raw file):

        
        except Exception as e:
            logger.exception(f"Failed to complete job {image_job.job_name} due to the following exception:")

Components developed more recently do:

        except Exception as e:
            logger.exception(f'Job failed due to: {e}')
            raise

Copy link
Contributor Author

@ZachCafego ZachCafego left a comment

Choose a reason for hiding this comment

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

Reviewable status: 2 of 3 files reviewed, 4 unresolved discussions (waiting on @jrobble)


python/ClipDetection/clip_component/clip_component.py line 31 at r2 (raw file):

Previously, jrobble (Jeff Robble) wrote…

VScode says Iterable and TypedDict are not used.

Done.


python/ClipDetection/clip_component/clip_component.py line 69 at r2 (raw file):

Previously, jrobble (Jeff Robble) wrote…

Components developed more recently do:

        except Exception as e:
            logger.exception(f'Job failed due to: {e}')
            raise

Done.


python/ClipDetection/clip_component/clip_component.py line 90 at r2 (raw file):

Previously, jrobble (Jeff Robble) wrote…

This should be -> Iterable[mpf.ImageLocation]:

Done.


python/ClipDetection/clip_component/clip_component.py line 95 at r2 (raw file):

Previously, jrobble (Jeff Robble) wrote…

Do the following in __init__:

self._preprocessor = None

Done.

Copy link
Member

@jrobble jrobble left a comment

Choose a reason for hiding this comment

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

Reviewed 1 of 1 files at r3, all commit messages.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @ZachCafego)

@ZachCafego ZachCafego merged commit 4893448 into develop Jul 7, 2023
@ZachCafego ZachCafego deleted the feat/clip-rollup branch July 7, 2023 19:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants