Skip to content

DOC - More fixes and improvements to the user guide#1574

Merged
GaelVaroquaux merged 57 commits into
skrub-data:mainfrom
rcap107:doc-userguide-updates
Sep 21, 2025
Merged

DOC - More fixes and improvements to the user guide#1574
GaelVaroquaux merged 57 commits into
skrub-data:mainfrom
rcap107:doc-userguide-updates

Conversation

@rcap107

@rcap107 rcap107 commented Aug 26, 2025

Copy link
Copy Markdown
Member

This PR is fixing a number of issues in the user guide and in some other parts of the documentation, and it reworks the overall structure of the user guide to make it easier for users to parse.

The main objective was to make it easier for readers to find what they are looking for by searching the left sidebar in the user guide. The current (as time of edit) user guide has a lot of content hidden behind titles that do not really describe the content of their section.

This PR addresses the problem by moving the content of the user guide from long files to a new modules/ folder, which contains a subfolder for each "chapter" of the user guide. Subfolders contain the old subsections.

The result is that now the sidebar shows the title of these subsections, making it easier for users to find information about the library.

The content of the guide has remained mostly the same: I reused most of what was already there, sometimes integrating the text with examples, or moving things around for clarity.

Additional changes include:

  • fixing all the broken references
  • rewriting the user guide intro
  • changing the user guide outline to go from simpler to more complex content
  • consistent use of "skrub" (rather than "Skrub" or skrub) throughout the docs
  • misc typo fixes
  • adding an example of a data ops report

@dholzmueller dholzmueller left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks, looks mostly good to me!

Comment thread doc/userguide_data_cleaning.rst Outdated
Comment thread doc/userguide_data_cleaning.rst Outdated
Comment thread examples/11_squashing_scaler.py Outdated
Comment thread examples/11_squashing_scaler.py
Comment thread examples/11_squashing_scaler.py Outdated
@rcap107

rcap107 commented Aug 27, 2025

Copy link
Copy Markdown
Member Author

In the end I decided to revamp most of the user guide, so this is a much bigger PR than I had originally intended

@rcap107

rcap107 commented Aug 27, 2025

Copy link
Copy Markdown
Member Author

Most of the file changes are actually moving stuff around

@rcap107

rcap107 commented Aug 27, 2025

Copy link
Copy Markdown
Member Author

Main changes:

  • I extended the section on the SquashingScaler and added links
  • I split the user guide in subsections so that it is easier to parse for the readers
  • I extended the section on the global config
  • I rewrote and clarified various sections
  • I fixed a few typos/formatting issues
  • I changed the destination of the "Learn more about Data Ops" in the website homepage so that it goes to the example rather than the user guide.

I still want to improve the Data Ops getting started example and user guide section.

@rcap107 rcap107 marked this pull request as ready for review August 27, 2025 10:28
@rcap107 rcap107 added the documentation Add or improve the documentation label Aug 27, 2025
@rcap107 rcap107 added this to the 0.6.2 milestone Aug 28, 2025
@GaelVaroquaux

GaelVaroquaux commented Sep 17, 2025 via email

Copy link
Copy Markdown
Member

@GaelVaroquaux

Copy link
Copy Markdown
Member

@rcap107 : can you resolve conflicts, in the mean time :)

@GaelVaroquaux

Copy link
Copy Markdown
Member

Start of a review performed in a broken way, when I find 3mn between meetings. Sorry, the review will be unstructured....

First point: isn't the link "[Discover the skrub DataOps →]" on the front page broken?

Maybe check links / run sphinx's linkcheck?

@rcap107

rcap107 commented Sep 17, 2025

Copy link
Copy Markdown
Member Author

Start of a review performed in a broken way, when I find 3mn between meetings. Sorry, the review will be unstructured....

First point: isn't the link "[Discover the skrub DataOps →]" on the front page broken?

Maybe check links / run sphinx's linkcheck?

it was broken because the numbering of the examples was changed

we really should move the dataops examples to 50+ or something, otherwise every time we change one of the previous examples we need to go and fix all the links

@GaelVaroquaux

Copy link
Copy Markdown
Member

we really should move the dataops examples to 50+ or something, otherwise every time we change one of the previous examples we need to go and fix all the links

That's a good idea! Do you want to have a go at it?

Comment thread doc/data_ops.rst Outdated
@rcap107

rcap107 commented Sep 17, 2025

Copy link
Copy Markdown
Member Author

we really should move the dataops examples to 50+ or something, otherwise every time we change one of the previous examples we need to go and fix all the links

That's a good idea! Do you want to have a go at it?

Only after this gets merged

Co-authored-by: Gael Varoquaux <gael.varoquaux@normalesup.org>
@@ -0,0 +1,10 @@
Column selection inside a pipeline
----------------------------------

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
----------------------------------
----------------------------------
.. currentmodule:: skrub

This is important so that the symbols actually turn into links to the API documentation (very important for the user)

@GaelVaroquaux

Copy link
Copy Markdown
Member

General comment: many of the :class:... entries are now broken. I did a suggestion to fix one (adding a "currentmodule") but we need to do it elsewhere, for instance here https://output.circle-artifacts.com/output/job/7c544d63-1587-438f-a73b-53ed1a926088/artifacts/0/doc/modules/joining_tables/interpolation_joiner.html

@rcap107 : could you review a bit the generated code to find and fix these issues. They are a quite severe regression

@rcap107

rcap107 commented Sep 17, 2025

Copy link
Copy Markdown
Member Author

General comment: many of the :class:... entries are now broken. I did a suggestion to fix one (adding a "currentmodule") but we need to do it elsewhere, for instance here https://output.circle-artifacts.com/output/job/7c544d63-1587-438f-a73b-53ed1a926088/artifacts/0/doc/modules/joining_tables/interpolation_joiner.html

@rcap107 : could you review a bit the generated code to find and fix these issues. They are a quite severe regression

I'll check all the files tomorrow

@jeromedockes jeromedockes left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

thanks a lot for all this @rcap107 , I'll continue reading it :)

Comment thread doc/_static/dataop_report/index.html Outdated
Comment thread doc/column_level_featurizing.rst
Comment thread doc/documentation.rst Outdated
Comment thread doc/documentation.rst Outdated
Comment thread doc/documentation.rst Outdated
Comment thread doc/modules/default_wrangling/table_vectorizer.rst Outdated
Comment thread doc/modules/default_wrangling/tabular_pipeline.rst
Comment thread doc/modules/default_wrangling/deduplicate_categorical_data.rst Outdated
Comment thread doc/column_level_featurizing.rst Outdated
Comment thread doc/modules/column_level_featurizing/feature_engineering_categorical.rst Outdated

@GaelVaroquaux GaelVaroquaux left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Many small suggestions mostly for formatting reasons.

My main comment is that the sections on DataOps goes too much in details, and people will not read through and thus not discover the features.

It should be made more high level, with mostly text and pointers, and the corresponding pages with a lot of code should be, IMHO, moved to examples.

I will apply all the suggested changes, and merge, unless I've broken something.

Comment thread doc/modules/column_level_featurizing/feature_engineering_datetimes.rst Outdated
Comment thread doc/modules/column_level_featurizing/feature_engineering_datetimes.rst Outdated
Comment thread doc/modules/multi_column_operations/multi_column_operations.rst Outdated
Comment thread doc/modules/multi_column_operations/drop_uninformative.rst
Comment thread doc/modules/multi_column_operations/selectors.rst Outdated
Comment thread doc/data_ops.rst

In this section we cover all about the skrub Data Ops, from starting out with a
simple example, to more advanced concepts like parameter tuning and and pipeline
validation.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The below does not render great in the site's table of contents (because the titles on this page are not exposed on the site's table of content) but I don't know how to fix this, so I'm moving on for now.

@@ -0,0 +1,94 @@
.. currentmodule:: skrub

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

That's a lot of code for a documentation page. I typically prefer moving these to an example, I find it easier to maintain.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The same comment applies to many of the pages in this section.

We could make a separate, dedicated gallery, that is inserted here, if useful

Comment thread doc/modules/data_ops/validation/hyperparameter_tuning.rst Outdated
Comment thread doc/modules/data_ops/validation/hyperparameter_tuning.rst
@GaelVaroquaux GaelVaroquaux merged commit 13bcc48 into skrub-data:main Sep 21, 2025
26 checks passed
rcap107 added a commit to rcap107/skrub that referenced this pull request Sep 25, 2025
Co-authored-by: Jérôme Dockès <jerome@dockes.org>
Co-authored-by: Gael Varoquaux <gael.varoquaux@normalesup.org>
rcap107 added a commit that referenced this pull request Sep 25, 2025
Co-authored-by: Jérôme Dockès <jerome@dockes.org>
Co-authored-by: Gael Varoquaux <gael.varoquaux@normalesup.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Add or improve the documentation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

DOC - Cross-links within skrub repo not working

4 participants