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

Add function to get version as of a certain date. Also formatting #378

Merged
merged 2 commits into from
Dec 16, 2022

Conversation

changhiskhan
Copy link
Contributor

No description provided.

@changhiskhan changhiskhan requested a review from eddyxu December 16, 2022 20:51
return _get_versioned_dataset(filesystem, uri, version)


def _is_versioned(filesystem, uri):
Copy link
Contributor

Choose a reason for hiding this comment

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

not sure what does this mean. it checks whether latest manifest exist? why it is related to versioned?

Or you mean it is a plain dataset? Should we just rename this function to is_plain_dataset?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

i'm just refactoring what you had in there

Copy link
Contributor Author

Choose a reason for hiding this comment

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

sure i can call this "is_plain_dataset"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

return not (filesystem.get_file_info(manifest).type == pa.fs.FileType.NotFound)


def _get_versioned_dataset(filesystem, uri, version):
Copy link
Contributor

Choose a reason for hiding this comment

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

let's add type annotations here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

return _dataset_plain(uri, filesystem=filesystem, **kwargs)

if asof is not None:
ds = dataset(uri, version, filesystem=filesystem, **kwargs)
Copy link
Contributor

Choose a reason for hiding this comment

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

use _get_versioned_dataset here? to reduce another I/O to check is_plain_dataset ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

@changhiskhan changhiskhan merged commit efc865d into main Dec 16, 2022
@changhiskhan changhiskhan deleted the changhiskhan/asof branch December 16, 2022 22:29
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.

2 participants