-
Notifications
You must be signed in to change notification settings - Fork 52
feat: add GeoSeries.from_xy()
#1364
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
Conversation
Check out this pull request on See visual diffs & provide feedback on Jupyter Notebooks. Powered by ReviewNB |
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.
Thanks!
Doctest failures:
These don't appear to be related to this change. |
Notebook test failures:
The error
|
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.
Per the notebook failure, please add some logic to ibis_dtype_to_bigframes_dtype
looking for all ibis geography-y types and returning the gpd.array.GeometryDtype()
.
Something like
if isinstance(ibis_dtype, ibis_dtypes.GeoSpatial):
return gpd.array.GeometryDtype()
around here:
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.
The reason we can look for ibis_dtypes.GeoSpatial
is that all the geography-y types have it as a superclass. See:
python-bigquery-dataframes/third_party/bigframes_vendored/ibis/expr/datatypes/core.py
Line 974 in b9bdca8
class Point(GeoSpatial): |
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.
Marking as request changes to cover the ibis_dtype_to_bigframes_dtype
change for the notebook fix before merging.
* chore: support timestamp subtractions * Fix format * use tree rewrites to dispatch timestamp_diff operator * add TODO for more node updates * polish the code and fix typos * fix comment * add rewrites to compile_raw and compile_peek_sql
* chore: add a tool to upload tpcds data to bigquery. * update error type * update docstring
* feat: add GeoSeries.from_xy * add from_xy test and update ibis types * update geoseries notebook with from_xy * Update docstring example * fix doctstring lint error * return GeometryDtype() for all ibis geo types * chore: support timestamp subtractions (#1346) * chore: support timestamp subtractions * Fix format * use tree rewrites to dispatch timestamp_diff operator * add TODO for more node updates * polish the code and fix typos * fix comment * add rewrites to compile_raw and compile_peek_sql * chore: add a tool to upload tpcds data to bigquery. (#1367) * chore: add a tool to upload tpcds data to bigquery. * update error type * update docstring --------- Co-authored-by: Shenyang Cai <sycai@users.noreply.github.com> Co-authored-by: Huan Chen <142538604+Genesis929@users.noreply.github.com>
Thank you for opening a Pull Request! Before submitting your PR, there are a few things you can do to make sure it goes smoothly:
from_xy
: screen/3pW4NWMsvfvVeYd, screen/3SimxdQU93zjinv, screen/rfsK45NBkxkUdz7Fixes #393394365 🦕