Disallow certain methods for dataset and tensor views #1311

FayazRahman · 2021-10-31T21:37:04Z

🚀 🚀 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

Closes #1163

CLAassistant · 2021-10-31T21:37:10Z

All committers have signed the CLA.

hub/util/exceptions.py

AbhinavTuli

Rename the error, resolve merge conflicts and then we should be good to merge this.

mikayelh · 2022-01-13T19:38:21Z

@tatevikh was this one resolved or is still in the backlog? asking to sort out the new contributor backlog!

…_view

codecov · 2022-01-20T04:45:15Z

Codecov Report

Merging #1311 (cdbdb33) into main (2893b40) will increase coverage by 0.47%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main    #1311      +/-   ##
==========================================
+ Coverage   91.75%   92.22%   +0.47%     
==========================================
  Files         193      194       +1     
  Lines       16879    17177     +298     
==========================================
+ Hits        15487    15842     +355     
+ Misses       1392     1335      -57

Flag	Coverage Δ
unittests	`92.22% <100.00%> (+0.47%)`	⬆️

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

Impacted Files	Coverage Δ
hub/api/tests/test_api.py	`100.00% <100.00%> (+1.74%)`	⬆️
hub/core/dataset/dataset.py	`93.27% <100.00%> (+0.57%)`	⬆️
hub/core/tensor.py	`84.28% <100.00%> (+1.20%)`	⬆️
hub/util/exceptions.py	`80.52% <100.00%> (+0.32%)`	⬆️
hub/util/invalid_view_op.py	`100.00% <100.00%> (ø)`
hub/core/compute/provider.py	`94.44% <0.00%> (-2.62%)`	⬇️
hub/integrations/tests/test_pytorch_dataloader.py	`95.69% <0.00%> (-1.98%)`	⬇️
hub/core/query/autocomplete.py	`79.54% <0.00%> (-1.71%)`	⬇️
hub/integrations/pytorch/dataset.py	`91.33% <0.00%> (-1.22%)`	⬇️
hub/core/meta/encode/chunk_id.py	`94.48% <0.00%> (-0.79%)`	⬇️
... and 62 more

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 2893b40...cdbdb33. Read the comment docs.

AbhinavTuli

Looks good!

jraman

Minor nits. Looks good otherwise.

hub/util/no_view.py

jraman · 2022-02-10T00:22:48Z

hub/util/no_view.py

+    @wraps(callable)
+    def inner(x, *args, **kwargs):
+        func = callable.__name__
+        if not x.index.is_trivial():


What if x does not have an index member? Or x.index doesn't have a is_trivial method?

this function is to be used internally, only for datasets and tensors

hub/util/exceptions.py

hub/util/no_view.py

farizrahman4u · 2022-03-15T09:02:12Z

hub/core/dataset/dataset.py

@@ -1048,6 +1055,7 @@ def size_approx(self):
        return size

    @hub_reporter.record_call
+    @invalid_view_op


I think this decorator should be above @hub_reporter.record_call for reporting to work properly.

FayazRahman added 3 commits November 1, 2021 01:51

init

f62cd39

add tests

9e69383

rm unwanted

e9e84da

FayazRahman added 2 commits November 1, 2021 03:22

mypy

ccfde3e

merge main

3edfdcb

tatevikh requested review from aliubimov and jraman November 16, 2021 09:11

aliubimov approved these changes Nov 17, 2021

View reviewed changes

aliubimov added the trigger-test Label trigger to run tests on PRs label Nov 17, 2021

activeloop-bot removed the trigger-test Label trigger to run tests on PRs label Nov 17, 2021

fix

6d17986

AbhinavTuli reviewed Dec 2, 2021

View reviewed changes

hub/util/exceptions.py Outdated Show resolved Hide resolved

AbhinavTuli suggested changes Dec 2, 2021

View reviewed changes

FayazRahman added 2 commits December 2, 2021 14:45

merge main

58a6fa1

rename error

ee4f076

AbhinavTuli added the trigger-test Label trigger to run tests on PRs label Dec 2, 2021

activeloop-bot removed the trigger-test Label trigger to run tests on PRs label Dec 2, 2021

pickle fix

d1dbaa0

FayazRahman requested a review from AbhinavTuli December 3, 2021 16:32

FayazRahman added 3 commits December 17, 2021 17:16

merge main + update

875a72a

remove unused

5530276

merge main + update

f897d92

FayazRahman added 3 commits January 14, 2022 13:22

merge main

93521ca

Merge branch 'main' of https://github.com/activeloopai/Hub into fy_no…

7342f66

…_view

update test

0b945e4

FayazRahman added the trigger-test Label trigger to run tests on PRs label Jan 20, 2022

activeloop-bot removed the trigger-test Label trigger to run tests on PRs label Jan 20, 2022

merge confl

c5b97ee

FayazRahman added the trigger-test Label trigger to run tests on PRs label Jan 23, 2022

activeloop-bot removed the trigger-test Label trigger to run tests on PRs label Jan 23, 2022

AbhinavTuli approved these changes Feb 8, 2022

View reviewed changes

jraman suggested changes Feb 10, 2022

View reviewed changes

FayazRahman added 5 commits February 13, 2022 13:16

merge main

63a1847

change decorator name + typing

37c2028

remove circular import

2ed077a

merge main

ba3bf35

merge confl

2e87da2

FayazRahman added the trigger-test Label trigger to run tests on PRs label Mar 15, 2022

activeloop-bot removed the trigger-test Label trigger to run tests on PRs label Mar 15, 2022

farizrahman4u suggested changes Mar 15, 2022

View reviewed changes

decorator order fix

cdbdb33

FayazRahman requested a review from farizrahman4u March 15, 2022 13:52

FayazRahman added the trigger-test Label trigger to run tests on PRs label Mar 15, 2022

activeloop-bot removed the trigger-test Label trigger to run tests on PRs label Mar 15, 2022

farizrahman4u approved these changes Mar 16, 2022

View reviewed changes

farizrahman4u merged commit 366d593 into activeloopai:main Mar 16, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Disallow certain methods for dataset and tensor views #1311

Disallow certain methods for dataset and tensor views #1311

FayazRahman commented Oct 31, 2021 •

edited

Loading

CLAassistant commented Oct 31, 2021 •

edited

Loading

AbhinavTuli left a comment

mikayelh commented Jan 13, 2022

codecov bot commented Jan 20, 2022 •

edited

Loading

AbhinavTuli left a comment

jraman left a comment

jraman Feb 10, 2022

FayazRahman Feb 13, 2022

farizrahman4u Mar 15, 2022

Disallow certain methods for dataset and tensor views #1311

Disallow certain methods for dataset and tensor views #1311

Conversation

FayazRahman commented Oct 31, 2021 • edited Loading

🚀 🚀 Pull Request

Checklist:

Changes

CLAassistant commented Oct 31, 2021 • edited Loading

AbhinavTuli left a comment

Choose a reason for hiding this comment

mikayelh commented Jan 13, 2022

codecov bot commented Jan 20, 2022 • edited Loading

Codecov Report

AbhinavTuli left a comment

Choose a reason for hiding this comment

jraman left a comment

Choose a reason for hiding this comment

jraman Feb 10, 2022

Choose a reason for hiding this comment

FayazRahman Feb 13, 2022

Choose a reason for hiding this comment

farizrahman4u Mar 15, 2022

Choose a reason for hiding this comment

FayazRahman commented Oct 31, 2021 •

edited

Loading

CLAassistant commented Oct 31, 2021 •

edited

Loading

codecov bot commented Jan 20, 2022 •

edited

Loading