-
Notifications
You must be signed in to change notification settings - Fork 251
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
Changhiskhan/datadiff #380
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm overall . small qq.
@@ -18,7 +18,7 @@ | |||
import pytz | |||
|
|||
import lance | |||
from lance.util.versioning import get_version_asof | |||
from lance.util.versioning import get_version_asof, compute_metric, diff, LanceDiff, RowDiff, ColumnDiff |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
isort.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
python/lance/util/versioning.py
Outdated
return pd.concat(results) | ||
|
||
|
||
def diff(uri, start_version: int, end_version: int) -> LanceDiff: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should we change the parameters to v1
and v2
?
start/end
makes me think this is find all versions from v1 to v2. like git log v1..v2
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sure. changed
401488b
to
6b5b885
Compare
This is on top of #379
Ignore the "compute_metric" stuff in this PR just look at the data diff changes