add width/height for dataset item #409
Conversation
jean-lucas
commented
Nov 9, 2023
jean-lucas
commented
- Allow passing width/height to dataset item
- Useful for privacy mode, where we cannot calculate width/height on our side
| def check_items_have_dimensions(dataset_items: Sequence[DatasetItem]): | ||
| for item in dataset_items: | ||
| has_width = getattr(item, "width") | ||
| has_height = getattr(item, "height") | ||
| if not (has_width and has_height): | ||
| raise Exception( | ||
| f"When using privacy mode, all items require a width and height. Missing for item: '{item.reference_id}'" | ||
| ) |
There was a problem hiding this comment.
fyi @gatli your script for privacy data upload will break after this change.
But i believe you can already get the image dimensions from the embedding service anyways?
jean-lucas
changed the title
gatli
left a comment
gatli
left a comment
There was a problem hiding this comment.
Can we lru_cache this or ideally just use the information passed for the constructor?
Otherwise LGTM 👍
nucleus/constants.py
Outdated
| DATASET_PRIVACY_MODE_KEY = "use_privacy_mode" | ||
| DATASET_SLICES_KEY = "slice_ids" | ||
| DATASET_USE_PRIVACY_MODE = "use_privacy_mode" |
There was a problem hiding this comment.
I'm seeing double 🧑🦲 🧑🦲 . Four "use_privacy_mode" constants!
There was a problem hiding this comment.
Where's my laugh? I make good joke! 😄
| @property | ||
| def use_privacy_mode(self) -> bool: | ||
| """Whether or not the dataset was created for privacy mode.""" | ||
| if self._use_privacy_mode is not None: | ||
| return self._use_privacy_mode | ||
| response = self._client.make_request( | ||
| {}, f"dataset/{self.id}/use_privacy_mode", requests.get | ||
| )[DATASET_USE_PRIVACY_MODE] | ||
| self._use_privacy_mode = response | ||
| return self._use_privacy_mode # type: ignore | ||
|
|
There was a problem hiding this comment.
I'm a little vary about using round-trip checks for every append. I think this will introduce more flakiness in our pipelines given how often we're seeing 503's etc.
Ideally we'd return this for the dataset constructor -> then we always have the data locally.
We should the very least cache this locally since this won't ever change.
There was a problem hiding this comment.
Good point, I changed client.create_dataset such that the properties name, is_scene, and use_privacy_mode are saved and dont require round-trip checks
| def check_items_have_dimensions(dataset_items: Sequence[DatasetItem]): | ||
| for item in dataset_items: | ||
| has_width = getattr(item, "width") | ||
| has_height = getattr(item, "height") | ||
| if not (has_width and has_height): | ||
| raise Exception( | ||
| f"When using privacy mode, all items require a width and height. Missing for item: '{item.reference_id}'" | ||
| ) |
pyproject.toml
Outdated
| [tool.poetry] | ||
| name = "scale-nucleus" | ||
| version = "0.16.7" | ||
| version = "0.16.9" |
There was a problem hiding this comment.
Nitpick, as I see, version 0.16.8 has not been released so maybe it would make sense to combine these changes into that version instead of creating a new one?
Opted to using the info passed during |
