Replace "DataSet" with "Dataset" in Markdown files

[deepyaman]

NOTE: Kedro datasets are moving from kedro.extras.datasets to a separate kedro-datasets package in
kedro-plugins repository. Any changes to the dataset implementations
should be done by opening a pull request in that repository.

Description

Development notes

I'll update in source files separately, if that works.

Note that any changes in kedro-datasets are entirely separate.

Checklist

• Read the contributing guidelines
• Opened this PR as a 'Draft Pull Request' if it is work-in-progress
• Updated the documentation to reflect the code changes
• Added a description of this change in the RELEASE.md file
• Added tests to cover my changes

[stichbury]

The changes are fine from my side because the docs are all building and I'm sure you've caught them all.

But I am still confused because if this drops in a release this week, what does that mean for users copying code snippets. Is the renaming done in the dataset package code already? (You mention doing the source separately so is that a no?). Does it mean the docs will be explaining a 0.19 change before it happens? Should this go into develop instead?

Also, the code in the starters will need updating too, correct?

Plus the graphic in the Data Catalog section on the website if we update the Spark Dataset

And blog posts. And also, what would be the strategy for third party datasets, and those coming from teams like the SparkStreaming dataset and the Databricks managed table one.

Sorry if I missed a meeting where this was discussed but while I'm supportive of this change, I don't understand/have missed how it's all tied together so we update everything at once. Otherwise we just end up with more inconsistency between those we've changed, those that haven't changed, and some that seem to be half-way changed.

[astrojuanlu]

Thanks for this push @stichbury. Indeed, I think we underestimated how difficult would all this be and particularly on the code side we've been blocked because of CI issues and unexpected things.

Let's try to be a bit strategic (maybe we can sync this afternoon or later this week) on how do we want to proceed.

[deepyaman]

Is the renaming done in the dataset package code already? (You mention doing the source separately so is that a no?).

I will do the stuff for source in kedro core repo shortly. 🤞 I've broken it up, also because I'm traveling, and larger PRs are harder.

Re dataset package, no, it will be done next (maybe this weekend? next week?). Since they're in separate repos, they do have to be done one by one, and I guess (as @astrojuanlu pointed out) it's been more challenging that anticipated.

[deepyaman]

@stichbury @astrojuanlu Actually, I was just wrapping up the changes in .py files, and it turns out we don't really have references to these in our docstrings? And I'll intentionally not change it in the tests to the non-deprecated version; can leave that as a test of deprecation, and update the tests, etc. once we've removed the old aliases for 0.19.

diff --git a/docs/source/conf.py b/docs/source/conf.py
index 5624e36e..22702666 100644
--- a/docs/source/conf.py
+++ b/docs/source/conf.py
@@ -348,16 +348,16 @@ def autolink_replacements(what: str) -> list[tuple[str, str, str]]:
     is a reStructuredText link to their documentation.
 
     For example, if the docstring reads:
-        This LambdaDataSet loads and saves ...
+        This LambdaDataset loads and saves ...
 
-    Then the word ``LambdaDataSet``, will be replaced by
-    :class:`~kedro.io.LambdaDataSet`
+    Then the word ``LambdaDataset``, will be replaced by
+    :class:`~kedro.io.LambdaDataset`
 
     Works for plural as well, e.g:
-        These ``LambdaDataSet``s load and save
+        These ``LambdaDataset``s load and save
 
     Will convert to:
-        These :class:`kedro.io.LambdaDataSet` load and save
+        These :class:`kedro.io.LambdaDataset` load and save
 
     Args:
         what: The objects to create replacement tuples for. Possible values

In the end, this was the only change I was going to make, but it doesn't seem to be important/not user-facing.

[astrojuanlu]

To @stichbury's points:

Sorry if I missed a meeting where this was discussed but while I'm supportive of this change, I don't understand/have missed how it's all tied together so we update everything at once.

I agree our efforts in this are have been kind of scattered. We're now tracking a more comprehensive plan in #2740

Also, the code in the starters will need updating too, correct?
Plus the graphic in the Data Catalog section on the website if we update the Spark Dataset

Right. I just added those two to the plan.

Is the renaming done in the dataset package code already?

I don't think this has been started yet.

But I am still confused because if this drops in a release this week, what does that mean for users copying code snippets.
And blog posts. And also, what would be the strategy for third party datasets, and those coming from teams like the SparkStreaming dataset and the Databricks managed table one.

I think these are very good points.

• If users find the new names but paste them in an old Kedro version, they'll get an unhelpful error. This is likely to happen, because many users will go to /en/stable/, rather than the appropriate version of the docs corresponding.
• Users might decide to use the new names, but they can't do it everywhere because the migration plan is incomplete, and inconsistencies remain.

The point that makes this more difficult is that part of these changes got already in main before we got a proper plan, which means that, as of now, old names will start emitting a DeprecationWarning in the latest version of Kedro. Reverting them is not impossible, but it can create some churn.

And on top of that, I contend that we should review our approach to these warnings, because as it stands now they can't even be silenced and it creates lots of noise on the Kedro CLI #2744

My proposal is:

• In the interest of avoiding having users that paste broken example code, let's hold off this pull request until we have clarity on what's needed for a proper migration ahead of 0.19, hence tackling Plan for Renaming DataSet to Dataset #2740
• In the interest of minimizing the warning noise (that creates user confusion but also CI madness), for the upcoming release (that most realistically would be shipped tomorrow) while not delaying the release of 0.18.11 any further, let's replace the DeprecationWarning that we're emitting with new names with a PendingDeprecationWarning, which is hopefully a small diff that buys us some time.

What do you think @deepyaman @noklam @stichbury ?

[deepyaman]

• In the interest of avoiding having users that paste broken example code, let's hold off this pull request until we have clarity on what's needed for a proper migration ahead of 0.19, hence tackling Plan for Renaming DataSet to Dataset #2740

What broken example code would they copy/paste? The idea is that the deprecated names should still work.

• In the interest of minimizing the warning noise (that creates user confusion but also CI madness), for the upcoming release (that most realistically would be shipped tomorrow) while not delaying the release of 0.18.11 any further, let's replace the DeprecationWarning that we're emitting with new names with a PendingDeprecationWarning, which is hopefully a small diff that buys us some time.

How does this help? Is it only because of what you mention in #2744? In that case, we can just remove the offending code (a holdover from the last time a bunch of datasets were deprecated for new names) instead of changing the warning type.

[astrojuanlu]

• In the interest of minimizing the warning noise (that creates user confusion but also CI madness), for the upcoming release (that most realistically would be shipped tomorrow) while not delaying the release of 0.18.11 any further, let's replace the DeprecationWarning that we're emitting with new names with a PendingDeprecationWarning, which is hopefully a small diff that buys us some time.

How does this help? Is it only because of what you mention in #2744? In that case, we can just remove the offending code (a holdover from the last time a bunch of datasets were deprecated for new names) instead of changing the warning type.

Yes, PendingDeprecationWarnings are hidden by default, and will basically be invisible. Addressing gh-2744 could be a more sound alternative and it wouldn't need that much code, it just requires some discussion.

[astrojuanlu]

What broken example code would they copy/paste?

That was a hasty summary: "Broken" if they paste new names in old Kedro versions. That of course can happen with any Kedro feature (or any other Python project for that matter) and I'm not saying that it should block the change - but without properly tackling the rest of gh-2740, there's a period in which we will have inconsistencies between the code, the docs, the blog posts, the images, etc, plus their kedro-datasets code will start emitting warnings they can't do anything about (the topic of kedro-org/kedro-plugins#255), plus they can't even selectively silence the warnings at all (gh-2744).

So, sure thing we need a transition period, but I don't think we need to rush the changes.

[deepyaman]

• In the interest of minimizing the warning noise (that creates user confusion but also CI madness), for the upcoming release (that most realistically would be shipped tomorrow) while not delaying the release of 0.18.11 any further, let's replace the DeprecationWarning that we're emitting with new names with a PendingDeprecationWarning, which is hopefully a small diff that buys us some time.

How does this help? Is it only because of what you mention in #2744? In that case, we can just remove the offending code (a holdover from the last time a bunch of datasets were deprecated for new names) instead of changing the warning type.

Yes, PendingDeprecationWarnings are hidden by default, and will basically be invisible. Addressing gh-2744 could be a more sound alternative and it wouldn't need that much code, it just requires some discussion.

Yeah, I think that's a more sound alternative, and I didn't realize it required more discussion (already had made a PR #2747 before I saw this 😅). I believe it's just a holdover from changes made ages ago.

[stichbury]

Just to confirm that I'm A-OK with these suggestions. Thank you for the summary @astrojuanlu and thanks for your patience @deepyaman

stale

[astrojuanlu]

Happy to land this when all the comments are addressed 👍🏽

[astrojuanlu]

RTD failure is unrelated

[deepyaman]

RTD failure is unrelated

@astrojuanlu Nice, I was wondering about that. Can this just be merged then?

[astrojuanlu]

I don't have permissions for it, but in any case I think the idea is to address the problem separately, see kedro-org/kedro-plugins#306

[astrojuanlu]

#2888 is going first, this will most likely need a rebase

[stichbury]

@deepyaman I have resolved a bunch of merge conflicts since my PR was merged first. Sorry but there may be some issues in one file the datasets examples file since this copied all the examples that were in data_catalog.md.

Please could you rescan that file just in case I missed any. Apologies again, but at least the messy merge is done...hopefully you can find and fix any capital 'S' swiftly.

[deepyaman]

@deepyaman I have resolved a bunch of merge conflicts since my PR was merged first. Sorry but there may be some issues in one file the datasets examples file since this copied all the examples that were in data_catalog.md.

Please could you rescan that file just in case I missed any. Apologies again, but at least the messy merge is done...hopefully you can find and fix any capital 'S' swiftly.

@stichbury Thanks for doing the hard work for me! I think I've caught (most of) the remaining changes.

find . -name '*.md' -print0 | xargs -0 sed -i "" "s/\([^A-Za-z]\)DataSet/\1Dataset/g"
find . -name '*.md' -print0 | xargs -0 sed -i "" "s/DataSetError/DatasetError/g"
find . -name '*.md' -print0 | xargs -0 sed -i "" "s/DataSetAlreadyExistsError/DatasetAlreadyExistsError/g"
find . -name '*.md' -print0 | xargs -0 sed -i "" "s/DataSetNotFoundError/DatasetNotFoundError/g"
find . -name '*.md' -print0 | xargs -0 sed -i "" "s/IncrementalDataSet/IncrementalDataset/g"
find . -name '*.md' -print0 | xargs -0 sed -i "" "s/PartitionedDataSet/PartitionedDataset/g"
find . -name '*.md' -print0 | xargs -0 sed -i "" "s/MemoryDataSet/MemoryDataset/g"
find . -name '*.md' -print0 | xargs -0 sed -i "" "s/LambdaDataSet/LambdaDataset/g"
find . -name '*.md' -print0 | xargs -0 sed -i "" "s/CachedDataSet/CachedDataset/g"
find . -name '*.md' -print0 | xargs -0 sed -i "" "s/AbstractDataSet/AbstractDataset/g"
find . -name '*.md' -print0 | xargs -0 sed -i "" "s/AbstractVersionedDataSet/AbstractVersionedDataset/g"
git checkout -- RELEASE.md

results in 76732ea

[stichbury]

All looks good to me!