Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

tensor.clear() to delete all samples from tensor #1288

Merged
merged 27 commits into from Mar 28, 2022
Merged

tensor.clear() to delete all samples from tensor #1288

merged 27 commits into from Mar 28, 2022

Conversation

FayazRahman
Copy link
Contributor

@FayazRahman FayazRahman commented Oct 24, 2021 •
edited

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

Added method to delete all samples from tensor.
Closes #1165.

@CLAassistant
Copy link

CLAassistant commented Oct 24, 2021 •
edited

CLA assistant check
All committers have signed the CLA.

@dhiganthrao dhiganthrao linked an issue Oct 25, 2021 that may be closed by this pull request
[FEATURE] Add method to delete all samples from tensor #1165
Closed
2 tasks
@tatevikh tatevikh requested review from jraman and removed request for verbose-void November 16, 2021 09:16
farizrahman4u
farizrahman4u suggested changes Nov 16, 2021
View reviewed changes
hub/core/chunk_engine.py Outdated Show resolved Hide resolved
hub/api/tests/test_chunk_sizes.py Show resolved Hide resolved
jraman
jraman suggested changes Nov 16, 2021
View reviewed changes
hub/core/chunk_engine.py Outdated Show resolved Hide resolved
hub/core/chunk_engine.py Outdated Show resolved Hide resolved
farizrahman4u
farizrahman4u previously approved these changes Nov 16, 2021
View reviewed changes
hub/core/chunk_engine.py Outdated Show resolved Hide resolved
hub/api/tests/test_api.py Show resolved Hide resolved
@farizrahman4u farizrahman4u dismissed their stale review November 16, 2021 19:02

approved by accident

@farizrahman4u farizrahman4u added the trigger-test Label trigger to run tests on PRs label Nov 22, 2021
@activeloop-bot activeloop-bot removed the trigger-test Label trigger to run tests on PRs label Nov 22, 2021
@codecov
Copy link

codecov bot commented Nov 22, 2021 •
edited

Codecov Report

Merging #1288 (32c4ec2) into main (536aa88) will increase coverage by 0.07%.
The diff coverage is 99.17%.

@@            Coverage Diff             @@
##             main    #1288      +/-   ##
==========================================
+ Coverage   92.37%   92.45%   +0.07%     
==========================================
  Files         200      200              
  Lines       17984    18202     +218     
==========================================
+ Hits        16613    16828     +215     
- Misses       1371     1374       +3
Flag Coverage Δ
unittests 92.45% <99.17%> (+0.07%) ⬆️

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

Impacted Files Coverage Δ
hub/core/tensor.py 84.76% <80.00%> (-0.17%) ⬇️
hub/api/tests/test_api.py 100.00% <100.00%> (ø)
hub/api/tests/test_chunk_sizes.py 100.00% <100.00%> (ø)
hub/core/chunk_engine.py 95.19% <100.00%> (+0.16%) ⬆️
hub/core/storage/gcs.py 75.11% <100.00%> (+0.46%) ⬆️
hub/core/storage/local.py 94.18% <100.00%> (+0.21%) ⬆️
hub/core/storage/lru_cache.py 96.27% <100.00%> (+0.18%) ⬆️
hub/core/storage/memory.py 100.00% <100.00%> (ø)
hub/core/storage/provider.py 100.00% <100.00%> (ø)
hub/core/storage/s3.py 69.28% <100.00%> (+0.11%) ⬆️
... and 7 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 536aa88...32c4ec2. Read the comment docs.

@mikayelh mikayelh mentioned this pull request Jan 13, 2022
Closed
2 tasks
@FayazRahman FayazRahman added the trigger-test Label trigger to run tests on PRs label Mar 21, 2022
@activeloop-bot activeloop-bot removed the trigger-test Label trigger to run tests on PRs label Mar 21, 2022
@FayazRahman FayazRahman added the trigger-test Label trigger to run tests on PRs label Mar 24, 2022
@activeloop-bot activeloop-bot removed the trigger-test Label trigger to run tests on PRs label Mar 24, 2022
AbhinavTuli
AbhinavTuli reviewed Mar 24, 2022
View reviewed changes
hub/core/version_control/commit_diff.py
@@ -55,6 +57,8 @@ def frombuffer(cls, data: bytes) -> "CommitDiff":
int.from_bytes(data[27 + i * 8 : 35 + i * 8], "big")
for i in range(num_updates)
}
pos = 35 + num_updates * 8
commit_diff.cleared = bool(int.from_bytes(data[pos : pos + 1], "big"))
Copy link
Contributor

Choose a reason for hiding this comment

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

this will not work for older datasets, should have an if-else condition here.

Copy link
Contributor

Choose a reason for hiding this comment

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

also, preferably have this boolean flag stored at the start along with the other booleans

Copy link
Contributor Author

Choose a reason for hiding this comment

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

this is fine right now since we are taking a slice of data. If we really want to move it up, what exactly will the if condition look like?

AbhinavTuli
AbhinavTuli reviewed Mar 24, 2022
View reviewed changes
hub/util/diff.py Outdated Show resolved Hide resolved
AbhinavTuli
AbhinavTuli suggested changes Mar 24, 2022
View reviewed changes
Copy link
Contributor

@AbhinavTuli AbhinavTuli left a comment

Choose a reason for hiding this comment

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

Some small changes, overall looks good!

@FayazRahman FayazRahman added the trigger-test Label trigger to run tests on PRs label Mar 25, 2022
@activeloop-bot activeloop-bot removed the trigger-test Label trigger to run tests on PRs label Mar 25, 2022
@FayazRahman FayazRahman added the trigger-test Label trigger to run tests on PRs label Mar 26, 2022
@activeloop-bot activeloop-bot removed the trigger-test Label trigger to run tests on PRs label Mar 26, 2022
@FayazRahman FayazRahman added the trigger-test Label trigger to run tests on PRs label Mar 27, 2022
@activeloop-bot activeloop-bot removed the trigger-test Label trigger to run tests on PRs label Mar 27, 2022
@FayazRahman FayazRahman merged commit ff63116 into activeloopai:main Mar 28, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jraman jraman jraman requested changes

@AbhinavTuli AbhinavTuli AbhinavTuli approved these changes

@aliubimov aliubimov Awaiting requested review from aliubimov

@farizrahman4u farizrahman4u Awaiting requested review from farizrahman4u

Assignees
No one assigned
Labels
hacktoberfest
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[FEATURE] Add method to delete all samples from tensor
7 participants
@FayazRahman @CLAassistant @jraman @farizrahman4u @AbhinavTuli @dhiganthrao @activeloop-bot