Skip to content

Conversation

@fxmarty
Copy link
Contributor

@fxmarty fxmarty commented May 12, 2022

Currently, there are no type hints nor Optional for the argument new_fingerprint in several methods of datasets.arrow_dataset.Dataset.

There was some documentation missing as well.

Note that pylance is happy with the type hints, but pyright does not detect that new_fingerprint is set within the decorator.

The modifications in this PR are fine since here

if inplace:
new_fingerprint = update_fingerprint(self._fingerprint, transform, kwargs_for_fingerprint)
else:
for fingerprint_name in fingerprint_names: # transforms like `train_test_split` have several hashes
if kwargs.get(fingerprint_name) is None:
kwargs_for_fingerprint["fingerprint_name"] = fingerprint_name
kwargs[fingerprint_name] = update_fingerprint(
self._fingerprint, transform, kwargs_for_fingerprint
)

for the non-inplace case we make sure to auto-generate a new fingerprint (as indicated in the doc).

@HuggingFaceDocBuilderDev
Copy link

HuggingFaceDocBuilderDev commented May 12, 2022

The documentation is not available anymore as the PR was closed or merged.

Copy link
Member

@lhoestq lhoestq left a comment

Choose a reason for hiding this comment

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

Good catch thanks ! LGTM :)

@lhoestq lhoestq merged commit 6382607 into huggingface:master Jun 1, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants