`remove`: output file/dir by path #2357

daavoo · 2021-04-06T07:19:04Z

content/docs/command-reference/remove.md

shcheklein

Thanks @daavoo ! That's an important change that we never had time to start making.

From reading this update (and not taking a look at the DVC PR) the way end users would read - I have a few questions. Mostly, it's not clear what semantics this operation has. I put some comments in the PR, it would be great if we could go and improve the docs/terminology.

daavoo · 2021-04-08T18:51:32Z

Thanks for the review, I realized that I need to change many more parts of the doc page in order to better explain the change.

jorgeorpinel · 2021-04-09T00:23:37Z

content/docs/command-reference/remove.md

+## Example: remove a single output file by name
+
+Imagine the same <abbr>workspace</abbr> as before but now we have multiple
+`outs` for the `train` stage:


Suggested change

## Example: remove a single output file by name

Imagine the same <abbr>workspace</abbr> as before but now we have multiple

`outs` for the `train` stage:

## Example: remove a stage output by file name

Imagine the we have multiple <abbr>outputs</abbr> for a `train` stage:

But why focus the example on stage outputs? What about making it about tracked data in general? Showing both removing an added dataset (dir), as well as a stage output (a model), for example. And/or update a previous example to mention the alternative to targeting .dvc files.

You are totally right, I was too focused on stage outputs because that was the "use case" described in the issue.

Happens. And we can go with this for now (just needs some fixes as indicated by Ivan) and I'll change it later. Unless you don't mind reworking it in a more general form along the way 🙂

Documenting this change is being definitely more challenging than implementing it xD.

@jorgeorpinel After reading iterative/dvc#2575 (comment) , I'm thinking that maybe we should remove the example dvc remove data.csv.dvc in favor of dvc remove data.csv, given that the behavior is now the same but new users might don't be aware of the concept of .dvc files?

The existing section ## Example: remove a tracked file kind of overlaps in content with the separate page # How to Stop Tracking Data.

Thanks for the updates @daavoo, I can wrap this up once iterative/dvc#5757 is merged.

Co-authored-by: Jorge Orpinel <jorgeorpinel@users.noreply.github.com>

daavoo · 2021-04-13T10:16:23Z

After trying to better explain the semantics I have now the feeling that the underlying problem might be that the current ui of dvc remove is capable of doing too many different things, as described by @jorgeorpinel in iterative/dvc#5791

shcheklein · 2021-04-14T03:00:16Z

content/docs/command-reference/remove.md

+/logs
+```
+
+The <abbr>output</abbr> file is actually being removed from the


@daavoo besides removing it from the .gitignore, does it mean that we have pretty much rm -f?

Apparently. Is that a bug @shcheklein ? It feels like it should be smart enough to edit dvc.yaml in this case.

shcheklein · 2021-04-14T03:02:09Z

content/docs/user-guide/how-to/stop-tracking-data.md

@@ -30,7 +30,7 @@ corresponding `.gitignore` entry). The data file is now no longer being tracked
 after this:

 ```dvc
-$ dvc remove data.csv.dvc
+$ dvc remove data.csv

 $ git status
    Untracked files:


it won't be untracked, right? it will become missing?

shcheklein · 2021-04-14T03:06:11Z

content/docs/command-reference/remove.md

+Note that, when using stage name as target, the actual <abbr>output</abbr> files
+or directories of the stage (`outs` field) are not removed by this command,
+unless the `--outs` option is used which will remove **all** of them.
+Alternatively, you can the names of individual <abbr>output</abbr> files or


but in this case we don't do anything to the .dvc or dvc.yaml?

this mixed semantics confuses me a bit to be honest

with some targets we touch dvc.yaml, but not data ... with some targets we touch data, but not DVC files

also, --outs that is not clear how it should be have when file/directory path is provided

It feels to me that even if we merge this (and the DVC core one), we'll need to get back to the whiteboard pretty soon with this dvc remove that we are long overdue cc @efiop @dberenbaum

Overall, not sure if this has enough value to do this now at least - it seems it's pretty much a replacement to rm -rf (+git command to remove .gitignore), unless I'm missing something? @daavoo what is your use case that you had in mind?

sorry, that we didn't get back to you initially in the DVC core ticket (or may be I missed the discussion).

Overall, not sure if this has enough value to do this now at least - it seems it's pretty much a replacement to rm -rf (+git command to remove .gitignore), unless I'm missing something? @daavoo what is your use case that you had in mind?

sorry, that we didn't get back to you initially in the DVC core ticket (or may be I missed the discussion).

I agree with the problem of mixed semantics @shcheklein . Sadly I don't think I'm in position to explain why (an intended use case) but rather how (what the new behavior is); I just picked a Good First Issue ticket to force myself to dive into the source code 😅 .

I think the original use cases were:

remove one of many stage outputs: misleading error message dvc#4497

ui: add a way to undo dvc add easily dvc#2575 (comment)

But maybe I misinterpreted the use cases along the implementation.

Yeah, sorry for the lack of clarity here after you've already put in so much work @daavoo.

but in this case we don't do anything to the .dvc or dvc.yaml?

I just tested, and it looks like .dvc is removed with this command but dvc.yaml won't be touched. Is that right @daavoo?

There might be enough straightforward utility here to merge the PR if it is limited to making dvc remove data.xml equivalent to dvc remove data.xml.dvc to support iterative/dvc#2575 (comment).

The rest might be valuable, too, but it seems like the semantics need to be straightened out in iterative/dvc#5791 first.

If you consider that we need to take a step back and put this and the dvc P.R. on pause or even discarding them in favor of waiting to find the right semantics, I would not mind it at all.

I wanted to dive into the source code and your thoughtful development process and I did; don't feel obligated to try to merge this just because of the work i already put.

dberenbaum · 2021-04-14T14:48:58Z

content/docs/command-reference/remove.md

 > `dvc remove` doesn't remove files from the DVC <abbr>cache</abbr> or
 > [remote storage](/doc/command-reference/remote). Use `dvc gc` for that.



I think it would be great to have an example (maybe just extending the one below) that shows how to remove the data from the cache, although I don't know that current gc functionality is sufficient to really be useful in this scenario.

content/docs/command-reference/remove.md

jorgeorpinel

Copy edits on the examples (will commit these)

content/docs/command-reference/remove.md

jorgeorpinel · 2021-04-27T23:06:39Z

content/docs/command-reference/remove.md

-## Example: remove a stage and its output
+## Example: remove a specific stage output


Not sure we need this example AND the next one ("remove specific data" from .dvc files)

jorgeorpinel · 2021-04-27T23:12:07Z

We can take this over once iterative/dvc#5757 is done. @daavoo if you don't mind just reminding us when/if that happens. Thanks!

dberenbaum · 2021-04-28T12:33:17Z

@jorgeorpinel Thanks for taking a look! I think this PR is on pause pending iterative/dvc#5791 (see #2357 (comment)).

shcheklein · 2021-07-16T00:29:47Z

Closing this as stale for now, let's get back to this when core repo is merged.

remove: Add example about removing single output file by name

cdf1bde

daavoo mentioned this pull request Apr 6, 2021

remove: Add support for removing tracked files / directories iterative/dvc#5757

Closed

2 tasks

Update remove.md

405dcf3

shcheklein reviewed Apr 6, 2021

View reviewed changes

content/docs/command-reference/remove.md Outdated Show resolved Hide resolved

shcheklein reviewed Apr 6, 2021

View reviewed changes

content/docs/command-reference/remove.md Show resolved Hide resolved

shcheklein reviewed Apr 6, 2021

View reviewed changes

content/docs/command-reference/remove.md Outdated Show resolved Hide resolved

shcheklein requested changes Apr 6, 2021

View reviewed changes

This was referenced Apr 7, 2021

ref: repro accepts outputs as targets #2362

Closed

repo: Add repro from given out names functionality iterative/dvc#5273

Closed

jorgeorpinel reviewed Apr 9, 2021

View reviewed changes

daavoo and others added 4 commits April 9, 2021 09:11

Update content/docs/command-reference/remove.md

63d864e

Co-authored-by: Jorge Orpinel <jorgeorpinel@users.noreply.github.com>

Update stop-tracking-data

d47b688

Update semantics

a4921ad

Add stage output files/directories

895cf55

daavoo requested review from jorgeorpinel and shcheklein April 13, 2021 10:16

shcheklein reviewed Apr 14, 2021

View reviewed changes

dberenbaum reviewed Apr 14, 2021

View reviewed changes

jorgeorpinel reviewed Apr 27, 2021

View reviewed changes

content/docs/command-reference/remove.md Outdated Show resolved Hide resolved

Update content/docs/command-reference/remove.md

ba1bdf5

jorgeorpinel reviewed Apr 27, 2021

View reviewed changes

Apply suggestions from code review

1daeb5f

restyled-io bot mentioned this pull request Apr 27, 2021

Restyle remove: output file/dir by path #2424

Closed

jorgeorpinel reviewed Apr 27, 2021

View reviewed changes

jorgeorpinel changed the title ~~remove: Add example about removing single output file by name~~ remove: output file/dir by path Apr 28, 2021

jorgeorpinel added the ⌛ status: wait-core-merge Waiting for related product PR merge/release label May 6, 2021

shcheklein closed this Jul 16, 2021

daavoo mentioned this pull request Jul 5, 2022

remove: support file as a target iterative/dvc#5288

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

`remove`: output file/dir by path #2357

`remove`: output file/dir by path #2357

daavoo commented Apr 6, 2021

shcheklein left a comment

daavoo commented Apr 8, 2021 •

edited by jorgeorpinel

Loading

jorgeorpinel Apr 9, 2021 •

edited

Loading

jorgeorpinel Apr 9, 2021

daavoo Apr 9, 2021 •

edited

Loading

jorgeorpinel Apr 10, 2021

daavoo Apr 13, 2021

daavoo Apr 13, 2021 •

edited

Loading

jorgeorpinel Apr 27, 2021

daavoo commented Apr 13, 2021 •

edited

Loading

shcheklein Apr 14, 2021

jorgeorpinel Apr 27, 2021

shcheklein Apr 14, 2021

shcheklein Apr 14, 2021

shcheklein Apr 14, 2021

daavoo Apr 14, 2021 •

edited

Loading

dberenbaum Apr 14, 2021

daavoo Apr 15, 2021 •

edited

Loading

dberenbaum Apr 14, 2021

jorgeorpinel left a comment

jorgeorpinel Apr 27, 2021

jorgeorpinel commented Apr 27, 2021

dberenbaum commented Apr 28, 2021

shcheklein commented Jul 16, 2021

		> `dvc remove` doesn't remove files from the DVC <abbr>cache</abbr> or
		> [remote storage](/doc/command-reference/remote). Use `dvc gc` for that.

		## Example: remove a stage and its output
		## Example: remove a specific stage output

remove: output file/dir by path #2357

remove: output file/dir by path #2357

Conversation

daavoo commented Apr 6, 2021

shcheklein left a comment

Choose a reason for hiding this comment

daavoo commented Apr 8, 2021 • edited by jorgeorpinel Loading

jorgeorpinel Apr 9, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

daavoo Apr 9, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

daavoo Apr 13, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

daavoo commented Apr 13, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

daavoo Apr 14, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

daavoo Apr 15, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jorgeorpinel left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jorgeorpinel commented Apr 27, 2021

dberenbaum commented Apr 28, 2021

shcheklein commented Jul 16, 2021

`remove`: output file/dir by path #2357

`remove`: output file/dir by path #2357

daavoo commented Apr 8, 2021 •

edited by jorgeorpinel

Loading

jorgeorpinel Apr 9, 2021 •

edited

Loading

daavoo Apr 9, 2021 •

edited

Loading

daavoo Apr 13, 2021 •

edited

Loading

daavoo commented Apr 13, 2021 •

edited

Loading

daavoo Apr 14, 2021 •

edited

Loading

daavoo Apr 15, 2021 •

edited

Loading