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.

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

Removed Assertions from shape_detector.py and added exceptions #616

Merged
merged 3 commits into from
Feb 28, 2021

Conversation

DebadityaPal
Copy link
Contributor

Some of the assertion errors were confusing, thus, explanations have been added. Solves #611

@github-actions
Copy link

Locust summary

Git references

Initial: cc22f2f
Terminal: 2c29de4

hub/store/shape_detector.py
Changes:
examples/large_dataset_build.py
Changes:

@AbhinavTuli
Copy link
Contributor

Looks good @DebadityaPal. Could you add a couple of unit tests for the new exceptions as well? Ideally, we want 100% codecov/patch at all times.

@codecov
Copy link

codecov bot commented Feb 25, 2021

Codecov Report

Merging #616 (4b09b4b) into master (cc22f2f) will increase coverage by 0.00%.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #616   +/-   ##
=======================================
  Coverage   89.32%   89.33%           
=======================================
  Files          54       54           
  Lines        3925     3927    +2     
=======================================
+ Hits         3506     3508    +2     
  Misses        419      419           
Impacted Files Coverage Δ
hub/store/shape_detector.py 96.96% <100.00%> (+0.06%) ⬆️

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 cc22f2f...4b09b4b. Read the comment docs.

@DebadityaPal
Copy link
Contributor Author

Yes, I should have added the unit tests, it escaped my notice. I'll do it right away.

@AbhinavTuli
Copy link
Contributor

@DebadityaPal you might want to check how to properly test exceptions using pytest. Here's an example from our tests

@DebadityaPal
Copy link
Contributor Author

@AbhinavTuli would it be helpful if I modified all the tests in the file to use the pytest format?

@AbhinavTuli AbhinavTuli linked an issue Feb 28, 2021 that may be closed by this pull request
@AbhinavTuli AbhinavTuli added the bug Something isn't working label Feb 28, 2021
@AbhinavTuli AbhinavTuli merged commit 3639a0c into activeloopai:master Feb 28, 2021
@AbhinavTuli
Copy link
Contributor

Just merged it. Thanks for yet another great PR @DebadityaPal !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[BUG] large_dataset_build.py falls with an error
2 participants