remove: support file as a target

[DiPaolo]

Description

As a further improvement of #4497, the support of using file as a target for dvc remove command should be added.

Here is what @skshetry says (https://github.com/iterative/dvc/pull/5236#issuecomment-759330299):

we need to support file as a target in the remove, so I am not sure if this new error message makes sense.

We do need to clean up the message, especially the "failed to remove" one. So, either we need to add support for it, or just do the cleanup in this PR.

Regarding the support for files, the change should be pretty trivial, we just need collect_granular here, similar to what's in the repo.commit:

dvc/dvc/repo/commit.py (Line 44 in 228b3b1): stages_info = self.stage.collect_granular( )

@efiop made another point (https://github.com/iterative/dvc/issues/4497#issuecomment-759298988) based on a user's feedback:

Another user ran into it https://discord.com/channels/485586884165107732/485596304961962003/798834932217937970

/cc @efiop @skshetry

[alexmojaki]

I was able to make this work with the following change to dvc/utils/__init__.py:

@@ -382,8 +382,12 @@ def parse_target(
                 )
             )
         if not name:
-            ret = (target, None)
-            return ret if is_valid_filename(target) else ret[::-1]
+            if is_valid_filename(target):
+                return target, None
+            elif os.path.isfile(target) and os.path.isfile(target + DVC_FILE_SUFFIX):
+                return target + DVC_FILE_SUFFIX, None
+            else:
+                return None, target

However it breaks the tests test_collect_granular_same_output_name_stage_name and test_gitignored_file_try_collect_granular_for_dvc_yaml_files. I'm guessing such a fundamental change has undesirable implications for the rest of the project. So should changes be isolated to dvc/repo/remove.py? Or is there somewhere else where it would be good to interpret a non-dvc file as a stage?

BTW, should dvc remove foo.bar just be equivalent to dvc remove foo.bar.dvc (when such a file exists) or should it do more, such as actually delete the file foo.bar?

[dberenbaum]

BTW, should dvc remove foo.bar just be equivalent to dvc remove foo.bar.dvc (when such a file exists) or should it do more, such as actually delete the file foo.bar?

It can just be equivalent to dvc remove foo.bar.dvc. There is already the --outs option to also delete foo.bar.

[daavoo]

Note that the last time I tackle this the conclusion was that dvc remove semantics were (more) unclear with this additional feature and decided to drop the P.R. (see https://github.com/iterative/dvc.org/pull/2357#issuecomment-828417049)

[dberenbaum]

Thanks for remembering that @daavoo. I would echo that it may not be worth the effort at the moment and may require more holistic changes to the UI.

[alexmojaki]

Here's a small change that only improves the error message: https://github.com/iterative/dvc/compare/main...alexmojaki:dvc:remove-file-message

Now when you run dvc remove thing, instead of:

ERROR: 'dvc.yaml' does not exist

it says

ERROR: 'thing' is not a .dvc file Do you mean 'thing.dvc'?: 'dvc.yaml' does not exist

and similarly instead of

ERROR: Stage 'thing' not found inside 'dvc.yaml' file

it says

ERROR: 'thing' is not a .dvc file Do you mean 'thing.dvc'?: Stage 'thing' not found inside 'dvc.yaml' file

[dberenbaum]

Great idea @alexmojaki! Let's go with that, thank you 🙏 .