Fix misleading add_column() usage example in docstring
Opened by ArjunJagdale (closed)

Fixes #7611

This PR fixes the usage example in the Dataset.add_column() docstring, which previously implied that add_column() modifies the dataset in-place.

Why:
The method returns a new dataset with the additional column, and users must assign the result to a variable to preserve the change.

This should make the behavior clearer for users.
@lhoestq @davanstrien 

Comments:
I believe there are other occurences of cases like this, like select_columns, select, filter, shard and flatten, could you also fix the docstring for them as well before we merge ?

Done! @lhoestq! I've updated the docstring examples for the following methods to clarify that they return new datasets instead of modifying in-place:

- `select_columns`
- `select`
- `filter`
- `shard`
- `flatten`


Also, any suggestions on what kind of issues I should work on next? I tried looking on my own, but I’d be happy if you could assign me something — I’ll do my best!


Hi! any update on this PR?

The docs for this PR live [here](https://moon-ci-docs.huggingface.co/docs/datasets/pr_7648). All of your documentation changes will be reflected on that endpoint. The docs are available until 30 days after the last update.

> Also, any suggestions on what kind of issues I should work on next? I tried looking on my own, but I’d be happy if you could assign me something — I’ll do my best!

Hmm. One long lasting issue is the one about being able to download only one split of a dataset (currently `load_dataset()` downloads all the splits, even when only one of train/test/validation is passed with `load_dataset(..., split=split)`)

This makes some downloads pretty long, I remember Mario started to work on this in this PR but couldn't finish it: https://github.com/huggingface/datasets/pull/6832

I think it would be a challenging but pretty impactful addition, and feel free to ping me if you have questions or if I can help. You can also take a look at Mario's first PR which was already in an advanced state. 

Let me know if it sounds like the kind of contribution you're looking for :)

Hi @lhoestq, thanks for the thoughtful suggestion!

The issue you mentioned sounds like a meaningful problem to tackle, and I’d love to take a closer look at it. I’ll start by reviewing Mario’s PR (#6832), understand what was implemented so far, and what remains to be done.

If I have any questions or run into anything unclear, I’ll be sure to reach out. 

I plan to give this a solid try. Thanks again — contributing to Hugging Face is something I truly hope to grow into.

---
Once again the the main Issue is to - 

>Allow users to download only the requested split(s) in load_dataset(...), avoiding unnecessary processing/downloading of the full dataset (especially important for large datasets like svhn, squad, glue).

right?

Also I have gone through some related / mentioned issues and PRs - 

- PR #6832 | Mario's main implementation for per-split download logic. Introduces splits param, _available_splits, and conditional logic in download_and_prepare()

- PR #6639 | Your earlier PR to trigger download_and_prepare() only when splits are missing from disk

- Issue #4101 / #2538 / #6529 | Real-world user complaints about load_dataset(..., split=...) still downloading everything. Confirm the need for this fix

- #2249 | Referenced by albertvillanova — old idea of caching only specific splits

---
IF I am not wrong, #2249 had some limitations - 
- Only worked for some dataset scripts where the download dict had split names as keys (like natural_questions).

- Would fail or cause confusing behavior on datasets with: 
1] Custom download keys (TRAIN_DOWNLOAD_URL, val_nyt, metadata)
2] Files passed one by one to dl_manager.download(), not as a dict

- Reused DownloadConfig, which led to blurry separation between cached_path, DownloadManager, and dataset logic.

- Needed to modify each dataset's _split_generators() to fully support split filtering.

- Risked partial or inconsistent caching if logic wasn’t tight.


> Hi @lhoestq, thanks for the thoughtful suggestion!
> 
> The issue you mentioned sounds like a meaningful problem to tackle, and I’d love to take a closer look at it. I’ll start by reviewing Mario’s PR (#6832), understand what was implemented so far, and what remains to be done.
> 
> If I have any questions or run into anything unclear, I’ll be sure to reach out.
> 
> I plan to give this a solid try. Thanks again — contributing to Hugging Face is something I truly hope to grow into.
> 
> Once again the the main Issue is to -
> 
> > Allow users to download only the requested split(s) in load_dataset(...), avoiding unnecessary processing/downloading of the full dataset (especially important for large datasets like svhn, squad, glue).
> 
> right?
> 
> Also I have gone through some related / mentioned issues and PRs -
> 
> * PR [Support downloading specific splits in `load_dataset` #6832](https://github.com/huggingface/datasets/pull/6832) | Mario's main implementation for per-split download logic. Introduces splits param, _available_splits, and conditional logic in download_and_prepare()
> * PR [Run download_and_prepare if missing splits #6639](https://github.com/huggingface/datasets/pull/6639) | Your earlier PR to trigger download_and_prepare() only when splits are missing from disk
> * Issue [How can I download only the train and test split for full numbers using load_dataset()?  #4101](https://github.com/huggingface/datasets/issues/4101) / [Loading partial dataset when debugging #2538](https://github.com/huggingface/datasets/issues/2538) / [Impossible to only download a test split #6529](https://github.com/huggingface/datasets/issues/6529) | Real-world user complaints about load_dataset(..., split=...) still downloading everything. Confirm the need for this fix
> * [Allow downloading/processing/caching only specific splits #2249](https://github.com/huggingface/datasets/pull/2249) | Referenced by albertvillanova — old idea of caching only specific splits
> 
> IF I am not wrong, #2249 had some limitations -
> 
> * Only worked for some dataset scripts where the download dict had split names as keys (like natural_questions).
> * Would fail or cause confusing behavior on datasets with:
>   1] Custom download keys (TRAIN_DOWNLOAD_URL, val_nyt, metadata)
>   2] Files passed one by one to dl_manager.download(), not as a dict
> * Reused DownloadConfig, which led to blurry separation between cached_path, DownloadManager, and dataset logic.
> * Needed to modify each dataset's _split_generators() to fully support split filtering.
> * Risked partial or inconsistent caching if logic wasn’t tight.

Also This one is in the charge now - https://github.com/huggingface/datasets/pull/7706#issue-3271129240