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

repro: update status message for dvc-added files #4233

Closed
sarthakforwet opened this issue Jul 17, 2020 · 6 comments
Closed

repro: update status message for dvc-added files #4233

sarthakforwet opened this issue Jul 17, 2020 · 6 comments
Labels
enhancement Enhances DVC p2-medium Medium priority, should be done, but less important ui user interface / interaction

Comments

@sarthakforwet
Copy link
Contributor

Bug Report

As we know that running dvc repro command would reproduce the stages defined in dvc.yaml file. One thing we also know that .dvc file can be used as the beginning of a pipeline. If we consider a sample case where we have the follwing pipeline and we perform dvc repro.

image

As we can see the command is successful in its execution but the thing to note that it's considering .dvc files as a stage (which they are probably not) as can be seen from the first line of the output.

Does the output corresponding to .dvc file needs some updations?
Output of dvc version:

$ dvc version
DVC version: 1.1.7
Python version: 3.8.3
Platform: Windows-10-10.0.19041-SP0
Binary: False
Package: pip
Supported remotes: http, https
Cache: reflink - not supported, hardlink - supported, symlink - supported
Repo: dvc, git

Additional Information (if any):

This issue is outcome of the discussion at #1572(review)

@triage-new-issues triage-new-issues bot added the triage Needs to be triaged label Jul 17, 2020
@sarthakforwet sarthakforwet changed the title Improve output for .dvc files corresponding to dvc repro command Improve output for .dvc file in dvc repro command Jul 17, 2020
@sarthakforwet sarthakforwet changed the title Improve output for .dvc file in dvc repro command Update output for .dvc file in dvc repro command Jul 17, 2020
@efiop
Copy link
Contributor

efiop commented Jul 18, 2020

Hi @sarthakforwet ! Great points! The dag part is discussed in #4058 , but we do indeed need to reconsider repro output as well. Thank you for the feedback!

@efiop efiop added the ui user interface / interaction label Jul 18, 2020
@triage-new-issues triage-new-issues bot removed the triage Needs to be triaged label Jul 18, 2020
@efiop efiop changed the title Update output for .dvc file in dvc repro command repro: update status message for dvc-added files Jul 18, 2020
@efiop efiop added enhancement Enhances DVC p3-nice-to-have It should be done this or next sprint p2-medium Medium priority, should be done, but less important and removed p3-nice-to-have It should be done this or next sprint labels Jul 18, 2020
@sarthakforwet
Copy link
Contributor Author

Thanks @efiop.

Suggestion: We can change the output so that whenever a .dvc extension is detected, it would print "Deps" instead of "Stage".

Also, Can I also work on this issue and what would be the prerequisite for it? Thanks

@efiop
Copy link
Contributor

efiop commented Jul 19, 2020

Suggestion: We can change the output so that whenever a .dvc extension is detected, it would print "Deps" instead of "Stage".

Good suggestion, but I think we are better off using something like

if not isinstance(stage, PipelineStage) and stage.is_data_source:
    # new message

🙂 Especially since .dvc might be a legacy pipeline file.

Also, Can I also work on this issue and what would be the prerequisite for it? Thanks

Sure! Please feel free. Looks like you'll simply need to adjust the message in

logger.info("Stage '%s' didn't change, skipping", self.addressing)
. Let us know if you'll have any questions. Thank you for looking into it! 🙏

@sarthakforwet
Copy link
Contributor Author

One doubt @efiop.
The "addressing" property has the following comment -

dvc/dvc/stage/__init__.py

Lines 165 to 166 in b77ce02

Useful for alternative presentations where we don't need
`Stage:` prefix.

and in "reproduce" we are adding the prefix "Stage" infront of it. So is it correct?

@efiop
Copy link
Contributor

efiop commented Jul 22, 2020

@sarthakforwet You can change the msg accordingly so it makes the most sense.

@efiop
Copy link
Contributor

efiop commented Dec 4, 2020

Fixed by #4317

@efiop efiop closed this as completed Dec 4, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Enhances DVC p2-medium Medium priority, should be done, but less important ui user interface / interaction
Projects
None yet
Development

No branches or pull requests

2 participants