Further testing

[zhong-al]

Resolves #74

• cvat2slowfast output
• cvat2ultralytics output
• detector2cvat output + mocked output
• miniscene2behavior mocked output (output testing in Load model from huggingface #76)
• player output
• tracks_extractor output

[NetZissou]

Optional suggestion:

flag was previously set to False. It now represents the negation of the no_images argument:

• When no_images is True, flag is False.
• When no_images is False, flag is True.

For clarity purpose, consider directly using the new argument no_images instead of introducing the flag variable, as flag is vague and less descriptive.

[zhong-al]

Can we put this in a new PR? I agree with this suggestion, but the diff in this one is already huge.

[NetZissou]

Sure thing!

[NetZissou]

Assuming the goal for clean_dir is to remove a directory regardless of its contents, you should use shutil.rmtree
(I made this mistake previously...)

import shutil
import os

def clean_dir(path):
    if os.path.exists(path):
        shutil.rmtree(path)

os.removedirs is designed to remove a directory and all of its parent directories only if they are empty. If the directory path contains files or subdirectories, this function will fail with an error.

[zhong-al]

I need some time to re-read how this is used. I might have had some reason for picking os.removedirs over shutil.rmtree.

[NetZissou]

Sounds good. Again sorry for the late response on this.

[zhong-al]

Oh, it's for a weird case in which I want to remove the empty parent directories. The slowfast model always creates an empty, nested output directory even though it doesn't output anything with the kabr inference config. I just added this function to avoid having random empty directories locally when testing.

[zhong-al]

No worries; thanks for the review! Do you have an improved name suggestion for this function? Maybe clean_empty_dirs? I can update & rebase on main after work today.

[NetZissou]

Yeah clean_empty_dirs looks good.

Check whether a directory is empty if possible, something like:

len(os.listdir(path)) == 0

And please summarize your last comment into docstrings or comment alongside the function. Thanks!

[zhong-al]

Rebasing the 111 commits is going to take more time than what I have tonight. I'll try to get it by next Monday!

Edit: I'll also apply both requested changes to cut down on the number of PRs & reviews :)

[zhong-al]

Ended up running git reset --soft HEAD~111 && git commit -m "Add further testing" and git rebase origin/master. Unsure if that + manually verifying conflicts was the best approach. Waiting to see if tests pass 🎉