Refactored Dataset Class #576

DebadityaPal · 2021-02-17T06:46:52Z

This PR is a temporary fix for the refactoring of the Dataset Class.

Since the Dataset class is too heavy and proper refactoring would take a significant amount of time and resources, this is a stopgap solution in which certain functions that convert hub datasets to other frameworks and vice-versa have been relocated from the base Dataset class file dataset.py to a new file integrations.py. These functions are not imported when a Dataset object is created, however, if the user calls these functions, they are subsequently imported and used.
Closes #526

github-actions · 2021-02-17T06:47:20Z

Locust summary

Git references

Initial: d9ebc7e
Terminal: a7a9f65

hub/api/integrations.py

Changes:

Name: from_tfds
Type: function
Changed lines: 205
Total lines: 205
Changes:
- Name: from_tfds.sampling
  Type: function
  Changed lines: 10
  Total lines: 10
- Name: from_tfds.dict_sampling
  Type: function
  Changed lines: 18
  Total lines: 18
- Name: from_tfds.generate_schema
  Type: function
  Changed lines: 3
  Total lines: 3
- Name: from_tfds.to_hub
  Type: function
  Changed lines: 22
  Total lines: 22
- Name: from_tfds.sdict_to_hub
  Type: function
  Changed lines: 7
  Total lines: 7
- Name: from_tfds.tensor_to_hub
  Type: function
  Changed lines: 12
  Total lines: 12
- Name: from_tfds.image_to_hub
  Type: function
  Changed lines: 13
  Total lines: 13
- Name: from_tfds.class_label_to_hub
  Type: function
  Changed lines: 7
  Total lines: 7
- Name: from_tfds.text_to_hub
  Type: function
  Changed lines: 4
  Total lines: 4
- Name: from_tfds.bbox_to_hub
  Type: function
  Changed lines: 3
  Total lines: 3
- Name: from_tfds.sequence_to_hub
  Type: function
  Changed lines: 2
  Total lines: 2
- Name: from_tfds.audio_to_hub
  Type: function
  Changed lines: 15
  Total lines: 15
- Name: from_tfds.video_to_hub
  Type: function
  Changed lines: 9
  Total lines: 9
- Name: from_tfds.transform_numpy
  Type: function
  Changed lines: 6
  Total lines: 6
- Name: from_tfds.my_transform
  Type: function
  Changed lines: 2
  Total lines: 2
Name: to_tensorflow
Type: function
Changed lines: 97
Total lines: 97
Changes:
- Name: to_tensorflow._get_active_item
  Type: function
  Changed lines: 13
  Total lines: 13
- Name: to_tensorflow.tf_gen
  Type: function
  Changed lines: 14
  Total lines: 14
- Name: to_tensorflow.dict_to_tf
  Type: function
  Changed lines: 5
  Total lines: 5
- Name: to_tensorflow.tensor_to_tf
  Type: function
  Changed lines: 2
  Total lines: 2
- Name: to_tensorflow.dtype_to_tf
  Type: function
  Changed lines: 9
  Total lines: 9
- Name: to_tensorflow.get_output_shapes
  Type: function
  Changed lines: 7
  Total lines: 7
- Name: to_tensorflow.output_shapes_from_dict
  Type: function
  Changed lines: 5
  Total lines: 5
Name: TorchDataset
Type: class
Changed lines: 81
Total lines: 81
Changes:
- Name: TorchDataset.__init__
  Type: function
  Changed lines: 11
  Total lines: 11
- Name: TorchDataset._do_transform
  Type: function
  Changed lines: 2
  Total lines: 2
- Name: TorchDataset._init_ds
  Type: function
  Changed lines: 14
  Total lines: 14
- Name: TorchDataset.__len__
  Type: function
  Changed lines: 3
  Total lines: 3
- Name: TorchDataset._get_active_item
  Type: function
  Changed lines: 13
  Total lines: 13
- Name: TorchDataset.__getitem__
  Type: function
  Changed lines: 27
  Total lines: 27
- Name: TorchDataset.__iter__
  Type: function
  Changed lines: 4
  Total lines: 4
Name: from_pytorch
Type: function
Changed lines: 91
Total lines: 91
Changes:
- Name: from_pytorch.sampling
  Type: function
  Changed lines: 3
  Total lines: 3
- Name: from_pytorch.dict_sampling
  Type: function
  Changed lines: 18
  Total lines: 18
- Name: from_pytorch.generate_schema
  Type: function
  Changed lines: 3
  Total lines: 3
- Name: from_pytorch.dict_to_hub
  Type: function
  Changed lines: 25
  Total lines: 25
- Name: from_pytorch.transform_numpy
  Type: function
  Changed lines: 6
  Total lines: 6
- Name: from_pytorch.my_transform
  Type: function
  Changed lines: 2
  Total lines: 2
Name: from_tensorflow
Type: function
Changed lines: 78
Total lines: 78
Changes:
- Name: from_tensorflow.generate_schema
  Type: function
  Changed lines: 4
  Total lines: 4
- Name: from_tensorflow.tf_to_hub
  Type: function
  Changed lines: 5
  Total lines: 5
- Name: from_tensorflow.TensorSpec_to_hub
  Type: function
  Changed lines: 4
  Total lines: 4
- Name: from_tensorflow.dict_to_hub
  Type: function
  Changed lines: 3
  Total lines: 3
- Name: from_tensorflow.transform_numpy
  Type: function
  Changed lines: 15
  Total lines: 15
- Name: from_tensorflow.my_transform
  Type: function
  Changed lines: 3
  Total lines: 3
Name: to_pytorch
Type: function
Changed lines: 33
Total lines: 33

hub/api/dataset.py

Changes:

Name: Dataset
Type: class
Changed lines: 15
Total lines: 917
Changes:
- Name: Dataset.to_pytorch
  Type: function
  Changed lines: 3
  Total lines: 24
- Name: Dataset.to_tensorflow
  Type: function
  Changed lines: 3
  Total lines: 15
- Name: Dataset.from_tensorflow
  Type: function
  Changed lines: 3
  Total lines: 31
- Name: Dataset.from_tfds
  Type: function
  Changed lines: 3
  Total lines: 37
- Name: Dataset.from_pytorch
  Type: function
  Changed lines: 3
  Total lines: 17

DebadityaPal · 2021-02-17T06:48:08Z

Also, @mynameisvinn should I keep the docstrings for the functions in both the files? Or would it be cleaner to remove them from dataset.py

codecov · 2021-02-17T06:51:29Z

Codecov Report

Merging #576 (6bceea7) into master (d9ebc7e) will increase coverage by 0.08%.
The diff coverage is 88.70%.

@@            Coverage Diff             @@
##           master     #576      +/-   ##
==========================================
+ Coverage   89.10%   89.18%   +0.08%     
==========================================
  Files          52       53       +1     
  Lines        3845     3874      +29     
==========================================
+ Hits         3426     3455      +29     
  Misses        419      419

Impacted Files	Coverage Δ
hub/api/integrations.py	`88.23% <88.23%> (ø)`
hub/api/dataset.py	`89.32% <100.00%> (+0.88%)`	⬆️

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 d9ebc7e...6bceea7. Read the comment docs.

AbhinavTuli · 2021-02-17T06:53:19Z

Looks good to me. I don't think we should remove the docstrings as we need them to show up when the user calls the function from Dataset class. The codecov/patch is a little short, other than that should be good to merge unless @mynameisvinn has any more suggestions.

DebadityaPal · 2021-02-17T07:08:04Z

I'll try to get better coverage for the patch

mynameisvinn · 2021-02-17T11:54:23Z

@DebadityaPal thanks, will review.

Docstrings. Always, always keep docstrings. This goes for modules, classes, and methods.
When you submit the updated PR, be sure to include a verbose description (why, what) since this is a stopgap solution.

mynameisvinn · 2021-02-18T13:10:42Z

@DebadityaPal two minor modifications.

Can you rename methods to_pytorch to _to_pytorch? This naming convention indicates that users should invoke to_pytorch from dataset.py, and not directly import to_pytorch from integrations.py.
Make sure __init__.py does not automatically import anything from integrations.py.

Then I think we're all set!

DebadityaPal · 2021-02-18T14:23:51Z

@mynameisvinn, @AbhinavTuli could you help me a bit with the codecov?
Most of the uncovered lines are ModuleNotFoundErrors .
However certain functions like the tf_gen() function in the _to_tensorflow() function, are being labelled as uncovered even though they are executed in the unit tests.
I put certain print statements in them to get a better sense of the situation, and they were printed indeed.

AbhinavTuli · 2021-02-18T15:51:18Z

Yeah, I'm aware of those functions not being covered. Our hypothesis is that codecov only covers the code that's on the main thread. I just took a look at the codecov and I think we can ignore the codecov/patch drop, it's not really on you, it just dropped because you happened to move the functions that were not covered, the codecov/project is going up anyway.

DebadityaPal added 2 commits February 17, 2021 11:45

Refactored Dataset class

c8d0845

return statements added to base functions, all pytest tests pass

a7a9f65

mynameisvinn requested review from mynameisvinn and AbhinavTuli February 17, 2021 11:54

mynameisvinn assigned mynameisvinn and DebadityaPal Feb 17, 2021

mynameisvinn added the enhancement New feature or request label Feb 17, 2021

Renamed functions in integrations.py

6bceea7

mynameisvinn merged commit 3746cde into activeloopai:master Feb 19, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Refactored Dataset Class #576

Refactored Dataset Class #576

DebadityaPal commented Feb 17, 2021 •

edited

github-actions bot commented Feb 17, 2021

DebadityaPal commented Feb 17, 2021

codecov bot commented Feb 17, 2021 •

edited

AbhinavTuli commented Feb 17, 2021

DebadityaPal commented Feb 17, 2021

mynameisvinn commented Feb 17, 2021

mynameisvinn commented Feb 18, 2021 •

edited

DebadityaPal commented Feb 18, 2021 •

edited

AbhinavTuli commented Feb 18, 2021

Refactored Dataset Class #576

Refactored Dataset Class #576

Conversation

DebadityaPal commented Feb 17, 2021 • edited

github-actions bot commented Feb 17, 2021

Locust summary

Git references

DebadityaPal commented Feb 17, 2021

codecov bot commented Feb 17, 2021 • edited

Codecov Report

AbhinavTuli commented Feb 17, 2021

DebadityaPal commented Feb 17, 2021

mynameisvinn commented Feb 17, 2021

mynameisvinn commented Feb 18, 2021 • edited

DebadityaPal commented Feb 18, 2021 • edited

AbhinavTuli commented Feb 18, 2021

DebadityaPal commented Feb 17, 2021 •

edited

codecov bot commented Feb 17, 2021 •

edited

mynameisvinn commented Feb 18, 2021 •

edited

DebadityaPal commented Feb 18, 2021 •

edited