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

changed links of rank2d and jointplotvisualizer #708

Closed
wants to merge 4 commits into from
Closed

changed links of rank2d and jointplotvisualizer #708

wants to merge 4 commits into from

Conversation

dnabanita7
Copy link
Contributor

absolute links are changed to relative docs links,to build it on go

bbengfort and others added 3 commits November 14, 2018 18:17
I changed the absolute links to relative docs links,so that its built on the go.
updated links of rank2d and jointplotvisualizer
Copy link
Member

@bbengfort bbengfort left a comment

Choose a reason for hiding this comment

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

Hi @naba7, thank you so much for the PR. I'm very sorry if I confused you with my suggestion of looking in the index.rst for the relative links; you faithfully followed my instructions! Rather, what I meant was that directives for the doc references for RankD and JointPlot were in the index.rst and could be copied and pasted into this document.

The goal here is that rather than using absolute links, e.g. https://mydomain.com/path/to/doc.html we use relative directives, e.g. :doc:`path/to/doc` so that when the documentation is built using make html in the documentation directly, or automatically by ReadTheDocs.org, the directive is replaced by the correct url, e.g. https://www.scikit-yb.org/latest/path/to/doc.html or https://www.scikit-yb.org/develop/path/to/doc.html, etc.

I've provided more specific instructions below in the hopes that you might become a more regular contributor. Would you mind making those changes?

Speaking of regularly contributing, I note that you're merging from the master branch of your fork. We use a branch-release cycle where our "main" branch is develop and only our stable releases are merged to master. In your fork if you git checkout develop you should be able to get up to date with the changes in our develop branch and we can merge your changes in that way. Sorry for the complication.

Thanks again for contributing the fix to those broken links and noticing them in the first place!

@@ -124,7 +124,7 @@ After downloading the dataset and unzipping it in your current working directory

The machine learning workflow is the art of creating *model selection triples*, a combination of features, algorithm, and hyperparameters that uniquely identifies a model fitted on a specific data set. As part of our feature selection, we want to identify features that have a linear relationship with each other, potentially introducing covariance into our model and breaking OLS (guiding us toward removing features or using regularization). We can use the Rank2D_ visualizer to compute Pearson correlations between all pairs of features as follows:

.. _Rank2D: http://www.scikit-yb.org/en/latest/api/yellowbrick.features.html#module-yellowbrick.features.rankd
.. _Rank2D: https://github.com/DistrictDataLabs/yellowbrick/blob/develop/docs/index.rst#id13
Copy link
Member

Choose a reason for hiding this comment

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

This line can be removed in favor of the edit above.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ok,sure I am correcting it

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you for being so patient and continuously helping me.I will be contributing regularly,because I like the organisation and I will be participating in gsoc 19

Copy link
Member

Choose a reason for hiding this comment

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

@naba7 - we're happy for your interest in scikit-yb and look forward to GSOC. I'm not seeing any new commits that correct the above edits suggested by @bbengfort -- is it possible that you accidentally pushed those commits to a different branch of your YB fork?

@@ -124,7 +124,7 @@ After downloading the dataset and unzipping it in your current working directory

The machine learning workflow is the art of creating *model selection triples*, a combination of features, algorithm, and hyperparameters that uniquely identifies a model fitted on a specific data set. As part of our feature selection, we want to identify features that have a linear relationship with each other, potentially introducing covariance into our model and breaking OLS (guiding us toward removing features or using regularization). We can use the Rank2D_ visualizer to compute Pearson correlations between all pairs of features as follows:
Copy link
Member

Choose a reason for hiding this comment

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

To reference the API documentation for RankD visualizers you could replace Rank2D_ with :doc:`Rank2D <api/features/rankd>` (note the backticks in the example) -- this allows us to use the sphinx.ext.intersphinx extension to reference other documents and you can remove the reference definition on line 127.

The problem with this is that this will go to the top of the RankD documentation page, and the section on Rank2d is farther down. In order to go to that specific section using vertical references you'd have to put a named anchor in that document and use :ref:`anchor-name`.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Can you please check if i did it correct?

Copy link
Member

Choose a reason for hiding this comment

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

Apologies @naba7 if we're missing something, but it doesn't appear that you've pushed the commits with the corrections that @bbengfort requested. Is it possible you pushed the changes to a different branch? If so, can I recommend that we close this PR and have you open a new PR from your documentation bugfix branch into our develop branch (this is our convention for contributors).

@@ -138,7 +138,7 @@ The machine learning workflow is the art of creating *model selection triples*,

This figure shows us the Pearson correlation between pairs of features such that each cell in the grid represents two features identified in order on the x and y axes and whose color displays the magnitude of the correlation. A Pearson correlation of 1.0 means that there is a strong positive, linear relationship between the pairs of variables and a value of -1.0 indicates a strong negative, linear relationship (a value of zero indicates no relationship). Therefore we are looking for dark red and dark blue boxes to identify further.

In this chart, we see that the features ``temp`` and ``feelslike`` have a strong correlation and also that the feature ``season`` has a strong correlation with the feature ``month``. This seems to make sense; the apparent temperature we feel outside depends on the actual temperature and other airquality factors, and the season of the year is described by the month! To dive in deeper, we can use the `JointPlotVisualizer <http://www.scikit-yb.org/en/latest/api/yellowbrick.features.html#module-yellowbrick.features.jointplot>`_ to inspect those relationships.
In this chart, we see that the features ``temp`` and ``feelslike`` have a strong correlation and also that the feature ``season`` has a strong correlation with the feature ``month``. This seems to make sense; the apparent temperature we feel outside depends on the actual temperature and other airquality factors, and the season of the year is described by the month! To dive in deeper, we can use the `JointPlotVisualizer https://github.com/DistrictDataLabs/yellowbrick/blob/develop/docs/index.rst#id27`_ to inspect those relationships.
Copy link
Member

Choose a reason for hiding this comment

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

Here too you could replace

`JointPlotVisualizer https://github.com/DistrictDataLabs/yellowbrick/blob/develop/docs/index.rst#id27`_

with

:doc:`Joint Plots <api/features/jointplot>`

And this would automatically create a link to the JointPlots documentation page!

@bbengfort bbengfort added the type: documentation writing and editing tasks for RTD label Jan 31, 2019
@dnabanita7
Copy link
Contributor Author

dnabanita7 commented Jan 31, 2019 via email

@rebeccabilbro
Copy link
Member

Ok thanks @naba7 - we'll keep an eye out for the new PR!

@rebeccabilbro rebeccabilbro added the invalid unrelated to primary development path label Feb 24, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
invalid unrelated to primary development path type: documentation writing and editing tasks for RTD
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants