[AL-1579] Auto htype #1370

farizrahman4u · 2021-12-02T12:54:03Z

🚀 🚀 Pull Request

Checklist:

My code follows the style guidelines of this project and the Contributing document
I have commented my code, particularly in hard-to-understand areas
I have kept the coverage-rate up
I have performed a self-review of my own code and resolved any problems
I have checked to ensure there aren't any other open Pull Requests for the same change
I have described and made corresponding changes to the relevant documentation
New and existing unit tests pass locally with my changes

Changes

Detect json and text htypes automatically. https://activeloop.atlassian.net/browse/AL-1579

CLAassistant · 2021-12-02T12:54:13Z

All committers have signed the CLA.

…r_auto_htype

jraman · 2021-12-13T19:50:59Z

hub/api/tests/test_api.py

@@ -811,3 +811,17 @@ def test_empty_extend(memory_ds):
        ds.create_tensor("y")
        ds.y.extend(np.zeros((len(ds), 3)))
    assert len(ds) == 0
+


Add test case for when set_htype is called and self.htype is not None.

set_htype is internal , analogous to set_dtype, that case shouldn't be hit from user space.

jraman · 2021-12-13T19:51:15Z

hub/api/tests/test_api.py

@@ -811,3 +811,17 @@ def test_empty_extend(memory_ds):
        ds.create_tensor("y")
        ds.y.extend(np.zeros((len(ds), 3)))
    assert len(ds) == 0
+


Add test case for when set_htype is called and self.length is > 0.

set_htype is internal , analogous to set_dtype, that case shouldn't be hit from user space.

jraman · 2021-12-13T19:52:41Z

hub/core/meta/tensor_meta.py

@@ -88,6 +80,38 @@ def set_dtype(self, dtype: np.dtype):

        self.dtype = dtype.name

+    def set_htype(self, htype: str, **kwargs):
+        """Should only be called once."""


More documentation please.

If self.htype is present, it must be None.

If self.length is present, it must be zero.

The docstr is similar to that of set_dtype. This is an internal method and the conditions are explicit in the checks.

jraman · 2021-12-13T19:56:34Z

hub/core/meta/tensor_meta.py

+        """Should only be called once."""
+        ffw_tensor_meta(self)
+
+        if getattr(self, "htype", None) is not None:


Add a test case when htype is not yet present.

This case is hit by default when create_tensor is called.

jraman · 2021-12-13T19:56:59Z

hub/core/meta/tensor_meta.py

+                f"Tensor meta already has a htype ({self.htype}). Incoming: {htype}."
+            )
+
+        if getattr(self, "length", 0) > 0:


Add a test case when length is not yet present.

This case is hit by default when create_tensor is called.

jraman · 2021-12-13T20:02:15Z

hub/util/casting.py

@@ -37,6 +37,21 @@ def get_dtype(val: Union[np.ndarray, Sequence, Sample]) -> np.dtype:
        raise TypeError(f"Cannot infer numpy dtype for {val}")


+def get_htype(val: Union[np.ndarray, Sequence, Sample]) -> str:
+    if isinstance(val, np.ndarray):


Add docstring.

jraman · 2021-12-13T20:02:58Z

hub/util/casting.py

@@ -37,6 +37,21 @@ def get_dtype(val: Union[np.ndarray, Sequence, Sample]) -> np.dtype:
        raise TypeError(f"Cannot infer numpy dtype for {val}")


+def get_htype(val: Union[np.ndarray, Sequence, Sample]) -> str:


samples instead of val is easier to parse for me.

Method is analogous to get_dtype.

jraman · 2021-12-13T20:04:08Z

hub/util/casting.py

+    if isinstance(val, np.ndarray):
+        return "generic"
+    types = set((map(type, val)))
+    if dict in types:


Don't all types have to be dict? Or, what's the case when not all are dicts (please add test case)?

Test added.

hub/util/casting.py

…r_auto_htype

codecov · 2021-12-23T08:20:14Z

Codecov Report

Merging #1370 (e36f021) into main (3ecaacb) will decrease coverage by 0.04%.
The diff coverage is 93.15%.

@@            Coverage Diff             @@
##             main    #1370      +/-   ##
==========================================
- Coverage   92.27%   92.22%   -0.05%     
==========================================
  Files         174      174              
  Lines       13850    13906      +56     
==========================================
+ Hits        12780    12825      +45     
- Misses       1070     1081      +11

Flag	Coverage Δ
unittests	`92.22% <93.15%> (-0.05%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
hub/core/serialize.py	`97.89% <66.66%> (-1.02%)`	⬇️
hub/core/meta/tensor_meta.py	`86.66% <92.00%> (-0.18%)`	⬇️
hub/util/casting.py	`84.12% <92.85%> (+2.49%)`	⬆️
hub/__init__.py	`96.96% <100.00%> (ø)`
hub/api/tests/test_api.py	`100.00% <100.00%> (ø)`
hub/core/chunk_engine.py	`96.66% <100.00%> (+0.01%)`	⬆️
hub/core/dataset/dataset.py	`93.52% <100.00%> (ø)`
hub/integrations/tests/test_pytorch_dataloader.py	`95.69% <0.00%> (-2.16%)`	⬇️
hub/integrations/pytorch/dataset.py	`91.49% <0.00%> (-1.62%)`	⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2d88131...e36f021. Read the comment docs.

auto htye

83ebe38

farizrahman4u added 2 commits December 7, 2021 18:15

Merge branch 'main' of https://www.github.com/activeloopai/hub into f…

d579fa9

…r_auto_htype

test

625c9a7

farizrahman4u marked this pull request as ready for review December 7, 2021 13:23

tatevikh requested review from jraman, AbhinavTuli and aliubimov and removed request for AbhinavTuli December 9, 2021 23:52

farizrahman4u added 2 commits December 13, 2021 08:39

Merge branch 'main' of https://www.github.com/activeloopai/hub into f…

1ae75af

…r_auto_htype

mypy

e0e038d

jraman suggested changes Dec 13, 2021

View reviewed changes

aliubimov approved these changes Dec 16, 2021

View reviewed changes

farizrahman4u added 3 commits December 22, 2021 17:50

nits

ebdc30c

more tests

cf44374

pass through test

ced86d1

tatevikh requested review from jraman and aliubimov December 22, 2021 15:31

jraman approved these changes Dec 22, 2021

View reviewed changes

farizrahman4u added 2 commits December 23, 2021 12:28

Merge branch 'main' of https://www.github.com/activeloopai/hub into f…

b3a59a2

…r_auto_htype

merge main

e36f021

farizrahman4u merged commit 83146b3 into main Dec 23, 2021

farizrahman4u deleted the fr_auto_htype branch December 23, 2021 09:35

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[AL-1579] Auto htype #1370

[AL-1579] Auto htype #1370

farizrahman4u commented Dec 2, 2021

CLAassistant commented Dec 2, 2021 •

edited

Loading

jraman Dec 13, 2021

farizrahman4u Dec 22, 2021

jraman Dec 13, 2021

farizrahman4u Dec 22, 2021

jraman Dec 13, 2021

farizrahman4u Dec 22, 2021

jraman Dec 13, 2021

farizrahman4u Dec 22, 2021

jraman Dec 13, 2021

farizrahman4u Dec 22, 2021

jraman Dec 13, 2021

jraman Dec 13, 2021

farizrahman4u Dec 22, 2021

jraman Dec 13, 2021

farizrahman4u Dec 22, 2021

codecov bot commented Dec 23, 2021 •

edited

Loading

		@@ -37,6 +37,21 @@ def get_dtype(val: Union[np.ndarray, Sequence, Sample]) -> np.dtype:
		raise TypeError(f"Cannot infer numpy dtype for {val}")


		def get_htype(val: Union[np.ndarray, Sequence, Sample]) -> str:

[AL-1579] Auto htype #1370

[AL-1579] Auto htype #1370

Conversation

farizrahman4u commented Dec 2, 2021

🚀 🚀 Pull Request

Checklist:

Changes

CLAassistant commented Dec 2, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov bot commented Dec 23, 2021 • edited Loading

Codecov Report

CLAassistant commented Dec 2, 2021 •

edited

Loading

codecov bot commented Dec 23, 2021 •

edited

Loading