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

Absorbing Understanding DVC #1320

Closed
wants to merge 2 commits into from
Closed

Absorbing Understanding DVC #1320

wants to merge 2 commits into from

Conversation

VANRao-Stack
Copy link

@VANRao-Stack VANRao-Stack commented May 19, 2020

I have deleted most of the content regarding explaination of core concepts and features and merged the more imortant part of this section into the user guide and the getting started section. Please do let me know if there are further changes that I would have to bring about. #144

You may disregard these recommendations if you used the Edit on GitHub button from dvc.org to improve a doc in place.

❗ Please read the guidelines in the Contributing to the Documentation list if you make any substantial changes to the documentation or JS engine.

🐛 Please make sure to mention Fix #issue (if applicable) in the description of the PR. This causes GitHub to close it automatically when the PR is merged.

Please choose to allow us to edit your branch when creating the PR.

Thank you for the contribution - we'll try to review it as soon as possible. 🙏

I have deleted most of the content regarding explaination of core concepts and features and merged the more imortant part of this section into the user guide and the getting started section. Please do let me know if there are further changes that I would have to bring about.
@jorgeorpinel
Copy link
Contributor

jorgeorpinel commented May 19, 2020

@jorgeorpinel jorgeorpinel temporarily deployed to dvc-landing-master-mmmumb36uxq May 19, 2020 17:42 Inactive
@jorgeorpinel
Copy link
Contributor

I have deleted most of the content regarding explaination of core concepts and features and merged the more important part of this section into the user guide and the getting started section.

Interesting. I'm creating a review app so we can see this PR live: https://dvc-landing-master-mmmumb36uxq.herokuapp.com/doc

But for starters I think that just moving how-it-works and resources as-is from understanding-dvc to user-guide doesn't really resolve the issue. See #425 — maybe focus only on this one subtask? #144 as a whole is too big.

@jorgeorpinel
Copy link
Contributor

moving how-it-works and resources as-is from understanding-dvc to user-guide

p.s. to really move them so they're listed in the navigation (see the preview I deployed but also please try to run this site locally) you would also have to update content/docs/sidebar.json accordingly 🙂 — but again, probably best not o simply move them, let's try to absorb all that content into existing User Guide or other (e.g. Get Started) pages.

Comment on lines 3 to 11
# Collaboration Issues in Data Science

Even with all the success we've seen today in machine learning (ML),
specifically deep learning and its applications in business, the data science
community still lacks good practices for organizing their projects and
effectively collaborating across their varied ML projects. This is a critical
challenge: we need to evolve towards ML algorithms and methods no longer being
tribal knowledge and making them easy to implement, reuse, and manage.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see this was copied verbatim from content/docs/understanding-dvc/collaboration-issues.md. It may be a good first step but this task (#425) needs a longer process of understanding the content we're absorbing, and adapting it to its new context. Some of this wording may be repetitive for example, not sure. But I'm almost positive it won't work to just copy-paste it. Please try reading it in its new context.

Comment on lines +3 to +18
Data Version Control, or DVC, is **a new type of experiment management
software** that has been built **on top of the existing engineering toolset that
you're already used to**, and particularly on a source code version control
system (currently Git). DVC reduces the gap between existing tools and data
science needs, allowing users to take advantage of experiment management
software while reusing existing skills and intuition.

The underlying source code control system eliminates the need to use external
services. Data science experiment sharing and collaboration can be done through
regular Git tools (commit messages, merges, pull requests, etc) the same way it
works for software engineers.

DVC implements a **Git experimentation methodology** where each experiment
exists with its code as well as data, and can be represented as a separate Git
branch or commit.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here (copy-pasted from content/docs/understanding-dvc/what-is-dvc.md)

@jorgeorpinel
Copy link
Contributor

I noticed you deleted much more content that the one moved to User Guide (from the deleted files). May I ask what criteria are we using to decide what stays and what goes? I'd like to understand your thought process here. Thanks @VANRao-Stack

@VANRao-Stack
Copy link
Author

VANRao-Stack commented May 19, 2020

I noticed you deleted much more content that the one moved to User Guide (from the deleted files). May I ask what criteria are we using to decide what stays and what goes? I'd like to understand your thought process here. Thanks @VANRao-Stack

So before this I had worked with Boost::Histogram and also with ROOT, and hence am moderately familiar with the two documentations. I noticed that both the documentations lack in descriptions about the two including details such as the core features and concepts, and mainly explained in brief the requirement of the said projects. I applied a similar kind of logic here, deleted most of the text going into the concepts of DVC and preserved those that give more meaningful insight into the same. Should I continue with the logic? Either way, I'll begin working on adapting the text that I copied into the new context.

@shcheklein shcheklein temporarily deployed to dvc-landing-master-mmmumb36uxq May 19, 2020 18:33 Inactive
@jorgeorpinel
Copy link
Contributor

before this I had worked with Boost::Histogram and also with ROOT

Interesting, can you please email or chat me references/links to your contributions or results for these projects? Thanks

deleted most of the text going into the concepts of DVC and preserved those that give more meaningful insight into the same. Should I continue with the logic?

The idea makes sense but it should be careful and thoughtful. I think probably there's relevant ideas in lots of the deleted text that should be preserved in other docs, unless you've checked that it's already redundant. Always keep in mind the goals of the issue (#425) expressed in it's description and some of the comments in there.

BTW still pending to link this PR to that issue, as mentioned in #1320 (comment)

@jorgeorpinel
Copy link
Contributor

Please re-request my review when ready for more feedback, @VANRao-Stack. Take your time.

image

@shcheklein
Copy link
Member

@VANRao-Stack First of all, thanks for trying to help us and being involved 🙏 I agree with @jorgeorpinel and I would recommend to go to the "whiteboard" and come up with a plan on what are we going to do with that content first. Right now PR looks quite aggressive and the place we moved these paragraphs not the ideal I would say. I'm closing this for now, since I expect pretty big changes will be required anyway.

@shcheklein shcheklein closed this May 19, 2020
@VANRao-Stack
Copy link
Author

@VANRao-Stack First of all, thanks for trying to help us and being involved 🙏 I agree with @jorgeorpinel and I would recommend to go to the "whiteboard" and come up with a plan on what are we going to do with that content first. Right now PR looks quite aggressive and the place we moved these paragraphs not the ideal I would say. I'm closing this for now, since I expect pretty big changes will be required anyway.

So should I continue working with it? Or should I find some other issue to contribute to? @shcheklein

@jorgeorpinel
Copy link
Contributor

jorgeorpinel commented May 20, 2020

Hey @VANRao-Stack you're welcome to work on this or pick a different issue. Part of the problem was that the one you found was a little miscategorized as a good-first-issue when really it was an epic containing some good first tasks. Since you're already familiar with this (issue #425) I would encourage you to try again but keep in mind even that smaller issue is relatively large. You could break it up in parts if you want and only address one of the parts.

It will just require a better strategy as mentioned in previous comments:

it should be careful and thoughtful... Always keep in mind the goals of the issue (#425) expressed in it's description and some of the comments in there.
"go back to the whiteboard" and come up with a plan on what are we going to do with that content first

Does that make sense?

@VANRao-Stack
Copy link
Author

Yes, I'll continue working on this itself then, but reorganise my work better this time without copying verbatim... Thanks, @jorgeorpinel

@jorgeorpinel
Copy link
Contributor

Thanks. Looking fwd to the 2nd attempt 🙂

@VANRao-Stack
Copy link
Author

VANRao-Stack commented May 22, 2020

Thanks. Looking fwd to the 2nd attempt

Yeah, I have been working on a new approach to the same issue, will make a PR when I'm ready! Also, I have been writing a couple of technical articles on Medium, you could check them out here https://medium.com/@varsg007

@shcheklein
Copy link
Member

@VANRao-Stack that sounds cool! But before you do a PR - I would recommend all of us to discuss the plan in the ticket itself - so that we are on the same page.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants