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-1433003 raise NotImplementedError for Index APIs #1750

Merged

Conversation

sfc-gh-vbudati
Copy link
Contributor

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

    Fixes SNOW-1433003

  2. Fill out the following pre-review checklist:

    • I am adding a new automated test(s) to verify correctness of my new code
      • If this test skips Local Testing mode, I'm requesting review from @snowflakedb/local-testing
    • I am adding new logging messages
    • I am adding a new telemetry message
    • I am adding new credentials
    • I am adding a new dependency
    • If this is a new feature/behavior, I'm adding the Local Testing parity changes.
  3. Please describe how your code solves the related issue.

    Raising NotImplementedError for Index APIs that have not been implemented. I also added documentation for them, and it should be tested.

@sfc-gh-vbudati sfc-gh-vbudati added NO-CHANGELOG-UPDATES This pull request does not need to update CHANGELOG.md NO-PANDAS-CHANGEDOC-UPDATES This PR does not update Snowpark pandas docs labels Jun 10, 2024
@sfc-gh-vbudati sfc-gh-vbudati requested a review from a team as a code owner June 10, 2024 17:58
Copy link
Contributor

@sfc-gh-nkrishna sfc-gh-nkrishna left a comment

Choose a reason for hiding this comment

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

Thanks for creating all these docstrings -- left a few comments. Can we also add an index.rst (okay if we do in a follow-up PR)?

@sfc-gh-vbudati
Copy link
Contributor Author

@sfc-gh-nkrishna there is a separate PR for documentation here: #1751
I want to get this one in before since the doc builder is not happy that the functions do not exist lol

except AttributeError as err:
if not key.startswith("_"):
native_index = native_pd.Index([])
if hasattr(native_index, key):
Copy link
Collaborator

Choose a reason for hiding this comment

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

@sfc-gh-vbudati can you add a quick comment here that "any methods that not supported by the current Index.py but exists in native pandas index object should raise a not implemented error for now"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ah I missed this - I'll add it in the documentation PR here: #1751

Copy link
Collaborator

Choose a reason for hiding this comment

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

Thanks!

@sfc-gh-vbudati sfc-gh-vbudati merged commit a3ef7f4 into main Jun 13, 2024
35 checks passed
@sfc-gh-vbudati sfc-gh-vbudati deleted the vbudati/SNOW-1433003-raise-not-implemented-error-index branch June 13, 2024 21:25
@github-actions github-actions bot locked and limited conversation to collaborators Jun 13, 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 NO-PANDAS-CHANGEDOC-UPDATES This PR does not update Snowpark pandas docs snowpark-pandas
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants