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

Fix ProfilerOptions() documentation #996

Closed
wants to merge 1 commit into from

Conversation

jacob-buehler
Copy link
Contributor

Fixed a bad link.

Issue #689:
The hyperlink in the below sentence in the Profiler options section of the Data Profiler documentation needs to link to better documentation for more in-depth options documentation.

@jacob-buehler jacob-buehler changed the base branch from main to dev August 2, 2023 20:33
@taylorfturner taylorfturner changed the title Fix ProfilerOptions() documentation, attempt no. 2 Fix ProfilerOptions() documentation Aug 2, 2023
@taylorfturner taylorfturner enabled auto-merge (squash) August 2, 2023 20:36
@@ -245,7 +245,7 @@
"\n",
"Below, let's remove the histogram and increase the number of samples to the labeler component (1,000 samples). \n",
"\n",
"Full list of options in the Profiler section of the [DataProfiler documentation](https://capitalone.github.io/DataProfiler)."
"Full list of options in the Profiler section of the [DataProfiler documentation](https://capitalone.github.io/DataProfiler/docs/0.10.2/html/profiler.html#profile-options)."
Copy link
Contributor

@taylorfturner taylorfturner Aug 2, 2023

Choose a reason for hiding this comment

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

I'm concerned with this change

Copy link
Contributor

@taylorfturner taylorfturner Aug 2, 2023

Choose a reason for hiding this comment

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

very brittle: because as soon as the version changes, this will break

Copy link
Contributor

Choose a reason for hiding this comment

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

just tested and still works with 0.10.1 @jacob-buehler @taylorfturner

Copy link
Contributor

Choose a reason for hiding this comment

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

....ah I see what u mean, breaks as in, it'll go stale

Copy link
Contributor

Choose a reason for hiding this comment

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

exactly -- goes stale, yes. Not that it breaks in the sense it doesn't go that versions link, but should go to the most up to date / max(version) of the docs

@@ -178,7 +178,7 @@
"\n",
"Below, let's remove the vocab count and set the stop words. \n",
"\n",
"Full list of options in the Profiler section of the [DataProfiler documentation](https://capitalone.github.io/DataProfiler)."
"Full list of options in the Profiler section of the [DataProfiler documentation](https://capitalone.github.io/DataProfiler/docs/0.10.2/html/profiler.html#profile-options)."
Copy link
Contributor

Choose a reason for hiding this comment

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

as above

@taylorfturner taylorfturner added the Documentation Improvements or additions to documentation label Aug 3, 2023
@jacob-buehler
Copy link
Contributor Author

updated in #1002

auto-merge was automatically disabled August 3, 2023 19:44

Pull request was closed

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

Successfully merging this pull request may close these issues.

3 participants