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

Fix model remove CLI command #805

Merged
merged 14 commits into from
Feb 10, 2023

Conversation

vinnamkim
Copy link
Contributor

@vinnamkim vinnamkim commented Feb 6, 2023

This should be merged after #804.

Summary

(datumaro) vinnamki@jisunkwo-desktop:~/datumaro/ws_test/logging$ datum model info
model-0
model-1
my-model
(datumaro) vinnamki@jisunkwo-desktop:~/datumaro/ws_test/logging$ datum model remove model-0
2023-02-06 10:33:01,318 ERROR: "Unknown model 'model-0'"
Traceback (most recent call last):
 File "/home/vinnamki/miniconda3/envs/datumaro/bin/datum", line 8, in <module>
   sys.exit(main())
 File "/home/vinnamki/datumaro/datumaro/cli/__main__.py", line 193, in main
   retcode = args.command(args)
 File "/home/vinnamki/datumaro/datumaro/util/scope.py", line 158, in wrapped_func
   ret_val = func(*args, **kwargs)
 File "/home/vinnamki/datumaro/datumaro/cli/contexts/model.py", line 171, in remove_command
   project.remove_model(args.name)
 File "/home/vinnamki/datumaro/datumaro/components/project.py", line 2689, in remove_model
   raise KeyError("Unknown model '%s'" % name)
KeyError: "Unknown model 'model-0'"

How to test

I added a test to cover this bug.

Checklist

License

  • I submit my code changes under the same MIT License that covers the project.
    Feel free to contact the maintainers if that's a concern.
  • I have updated the license header for each file (see an example below)
# Copyright (C) 2021 Intel Corporation
#
# SPDX-License-Identifier: MIT

vinnamkim and others added 10 commits February 3, 2023 19:02
 - Fix model loading in datum explain
 - Add an integration test for datum explain

Signed-off-by: Kim, Vinnam <vinnam.kim@intel.com>
Signed-off-by: Kim, Vinnam <vinnam.kim@intel.com>
Signed-off-by: Kim, Vinnam <vinnam.kim@intel.com>
Signed-off-by: Kim, Vinnam <vinnam.kim@intel.com>
Signed-off-by: Kim, Vinnam <vinnam.kim@intel.com>
Signed-off-by: Kim, Vinnam <vinnam.kim@intel.com>
Signed-off-by: Kim, Vinnam <vinnam.kim@intel.com>
 - Fix datum model info description too

Signed-off-by: Kim, Vinnam <vinnam.kim@intel.com>
Signed-off-by: Kim, Vinnam <vinnam.kim@intel.com>
@vinnamkim vinnamkim changed the title Fix/model remove Fix model remove command Feb 6, 2023
@vinnamkim vinnamkim changed the title Fix model remove command Fix model remove CLI command Feb 6, 2023
Signed-off-by: Kim, Vinnam <vinnam.kim@intel.com>
@vinnamkim vinnamkim added BUG Something isn't working cli labels Feb 6, 2023
@vinnamkim vinnamkim marked this pull request as ready for review February 9, 2023 01:08
@codecov-commenter
Copy link

codecov-commenter commented Feb 9, 2023

Codecov Report

Base: 77.67% // Head: 77.67% // Decreases project coverage by -0.01% ⚠️

Coverage data is based on head (fccaed5) compared to base (b10ee42).
Patch coverage: 0.00% of modified lines in pull request are covered.

Additional details and impacted files
@@             Coverage Diff             @@
##           develop     #805      +/-   ##
===========================================
- Coverage    77.67%   77.67%   -0.01%     
===========================================
  Files          179      179              
  Lines        23335    23337       +2     
  Branches      5032     5032              
===========================================
  Hits         18126    18126              
- Misses        4146     4148       +2     
  Partials      1063     1063              
Flag Coverage Δ
macos-11_Python-3.10 77.59% <0.00%> (-0.01%) ⬇️
macos-11_Python-3.7 77.16% <0.00%> (-0.01%) ⬇️
macos-11_Python-3.8 77.53% <0.00%> (?)
macos-11_Python-3.9 77.55% <0.00%> (?)
ubuntu-20.04_Python-3.10 77.59% <0.00%> (-0.01%) ⬇️
ubuntu-20.04_Python-3.7 77.16% <0.00%> (-0.01%) ⬇️
ubuntu-20.04_Python-3.8 77.54% <0.00%> (-0.01%) ⬇️
ubuntu-20.04_Python-3.9 77.55% <0.00%> (-0.01%) ⬇️
windows-2019_Python-3.10 77.53% <0.00%> (?)
windows-2019_Python-3.7 77.10% <0.00%> (-0.01%) ⬇️
windows-2019_Python-3.8 77.47% <0.00%> (?)
windows-2019_Python-3.9 77.48% <0.00%> (-0.01%) ⬇️

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

Impacted Files Coverage Δ
datumaro/cli/contexts/model.py 21.64% <0.00%> (-0.17%) ⬇️
datumaro/components/project.py 79.18% <0.00%> (-0.06%) ⬇️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

wonjuleee
wonjuleee previously approved these changes Feb 10, 2023
Copy link
Contributor

@wonjuleee wonjuleee left a comment

Choose a reason for hiding this comment

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

Looks good to me! Thank you for your contribution.

@vinnamkim vinnamkim added this pull request to the merge queue Feb 10, 2023
Merged via the queue into openvinotoolkit:develop with commit b4a396b Feb 10, 2023
@vinnamkim vinnamkim added this to the 1.0.0 milestone Feb 17, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
BUG Something isn't working cli
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants