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

SNOW-1316977: Remove fallbacks and raise not implemented error #1435

Merged
merged 1 commit into from
Apr 30, 2024

Conversation

sfc-gh-nkumar
Copy link
Contributor

@sfc-gh-nkumar sfc-gh-nkumar commented Apr 26, 2024

Please answer these questions before submitting your pull requests. Thanks!

  1. What GitHub issue is this PR addressing? Make sure that there is an accompanying issue to your PR.

    Fixes SNOW-1316977

  2. Fill out the following pre-review checklist:

    • I am adding a new automated test(s) to verify correctness of my new code
    • I am adding new logging messages
    • I am adding a new telemetry message
    • I am adding new credentials
    • I am adding a new dependency
  3. Please describe how your code solves the related issue.

    Remove fallback for all dataframe and series APIs. This list is primarily driven from https://docs.google.com/document/d/1uMwNgLqFhtoAFeEj59XjR3uKQ77QjA84aTmv8Erbvw0/edit#heading=h.qhdbmdhgqdh5. There still might be more fallback which are not documented. This will be cleaned up in https://snowflakecomputing.atlassian.net/browse/SNOW-1347394
    NOTE: I have kept the existing tests mostly unmodified even though there is redundancy. This can be useful when we implement these APIs.
    NOTE: This also require updates in docs/*supported.rst files. These files are not yet ported from snowpandas. So will do a follow up PR to update the docs.

Copy link

github-actions bot commented Apr 26, 2024

CLA Assistant Lite bot All contributors have signed the CLA ✍️ ✅

@sfc-gh-nkumar sfc-gh-nkumar force-pushed the nkumar-SNOW-1316977-fallback branch from c4347f4 to 8b55d36 Compare April 26, 2024 03:44
@sfc-gh-nkumar
Copy link
Contributor Author

I have read the CLA Document and I hereby sign the CLA

@sfc-gh-nkumar sfc-gh-nkumar force-pushed the nkumar-SNOW-1316977-fallback branch 2 times, most recently from 5f2c209 to df2ae49 Compare April 26, 2024 18:58
@sfc-gh-nkumar sfc-gh-nkumar marked this pull request as ready for review April 26, 2024 19:14
@sfc-gh-nkumar sfc-gh-nkumar requested a review from a team as a code owner April 26, 2024 19:14
@sfc-gh-nkumar sfc-gh-nkumar changed the title [DRAFT] SNOW-1316977: Remove fallbacks ans raise not implemented error instead SNOW-1316977: Remove fallbacks and raise not implemented error Apr 26, 2024
@sfc-gh-nkumar sfc-gh-nkumar force-pushed the nkumar-SNOW-1316977-fallback branch 3 times, most recently from 4093765 to 4247a8e Compare April 29, 2024 17:16
@sfc-gh-nkumar sfc-gh-nkumar added the NO-CHANGELOG-UPDATES This pull request does not need to update CHANGELOG.md label Apr 29, 2024
Copy link
Contributor

@sfc-gh-helmeleegy sfc-gh-helmeleegy left a comment

Choose a reason for hiding this comment

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

Overall, LGTM! Just left a few nits.

@sfc-gh-nkumar sfc-gh-nkumar merged commit 1309d80 into main Apr 30, 2024
23 of 24 checks passed
@sfc-gh-nkumar sfc-gh-nkumar deleted the nkumar-SNOW-1316977-fallback branch April 30, 2024 05:07
@github-actions github-actions bot locked and limited conversation to collaborators Apr 30, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
NO-CHANGELOG-UPDATES This pull request does not need to update CHANGELOG.md
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants