`metrics remove` doesn't work for pipeline files

[Suor]

The dvc metrics remove command doesn't work if targeted metric resides within pipeline file. The underlying issue is that Dvcfile.dump() might dump a file or not depending on particular class.

Looks like the culprit is that PipelineFile.dump() tries to serve two purposes, i.e. dump both pipeline file and corresponding lock file, additionally PipelineFile.dump() has unsafe default, i.e. not really saving a pipeline file unless extra flag is passed. I would split repsponsibilities:

1. Make PipelineFile.dump() dump pipeline file as its name suggest.
2. Add PipelineFile.dump_lock() and call that when you need to dump a lock file.

NOTE: I found this bug by spotting the unsafe default in .dump() and looking through its uses. There might be more related bugs.

Output of dvc version:

$ dvc version
DVC version: 1.0.0a6+bb50fb
Python version: 3.8.2
Platform: Linux-5.4.0-31-generic-x86_64-with-glibc2.27
Binary: False
Package: None
Supported remotes: http, https
Cache: reflink - not supported, hardlink - supported, symlink - supported
Repo: dvc, git

[skshetry]

Related to #3849.

Also, we are deprecating some commands from metrics including remove.

additionally PipelineFile.dump() has unsafe default, i.e. not really saving a pipeline file unless extra flag is passed

It does that to be safe, i.e. it should not try to write to PipelineFile as much as possible, and provide a flag wherever we need to.

[efiop]

@skshetry Not deprecating these if we can fix them. We will have to introduce them later anyways. Will see in #3849 if we could fix them both.

[Suor]

It does that to be safe, i.e. it should not try to write to PipelineFile as much as possible, and provide a flag wherever we need to.

This is unsafe in a sense that the function doesn't always do what it is expected to do. This also breaks abstraction, i.e. other classes use that to save file, but this one doesn't. At least one bug was caused by this. If we need a function that doesn't dump a file, we should call it something else but .dump().

Did anybody check that no other bugs like this are there, for example? Even if we don't have now we might have some in the future. People will rightfully expect that .dump() saves file and use it like that.

[Suor]

The very least we can do is reverse the default, so PipelineFile.dump() will dump it unless some flag says otherwise. This would be still a so-so design, but at least not as error prone.