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.

Sign up for GitHub

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

Jump to bottom

SNOW-1805842: Add plotly integ tests and Interoperability doc #2725

Merged
merged 18 commits into from
Dec 14, 2024
Merged

SNOW-1805842: Add plotly integ tests and Interoperability doc #2725

merged 18 commits into from
Dec 14, 2024

Conversation

sfc-gh-lmukhopadhyay
Copy link
Contributor

@sfc-gh-lmukhopadhyay sfc-gh-lmukhopadhyay commented Dec 6, 2024 •
edited
Loading

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

    Fixes SNOW-1805842

  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.
    • I acknowledge that I have ensured my changes to be thread-safe. Follow the link for more information: Thread-safe Developer Guidelines

  3. Please describe how your code solves the related issue.

    Adding Plotly express interoperability tests and a docs page for guaranteed interoperable APIs.

@sfc-gh-lmukhopadhyay
SNOW-1805842: add plotly integ tests and interop doc
539799d 
Signed-off-by: Labanya Mukhopadhyay <labanya.mukhopadhyay@snowflake.com>
@sfc-gh-lmukhopadhyay
update setup for plotly dependency
12ee8aa 
Signed-off-by: Labanya Mukhopadhyay <labanya.mukhopadhyay@snowflake.com>
@sfc-gh-lmukhopadhyay sfc-gh-lmukhopadhyay added the NO-CHANGELOG-UPDATES This pull request does not need to update CHANGELOG.md label Dec 6, 2024
@sfc-gh-lmukhopadhyay
move and update doc
6f27bbd 
Signed-off-by: Labanya Mukhopadhyay <labanya.mukhopadhyay@snowflake.com>
@sfc-gh-lmukhopadhyay
Merge branch 'main' into lmukhopadhyay-SNOW-1805842-add-plotly-integ-…
aecb735 
…tests
@sfc-gh-lmukhopadhyay sfc-gh-lmukhopadhyay marked this pull request as ready for review December 7, 2024 00:52
@sfc-gh-lmukhopadhyay sfc-gh-lmukhopadhyay requested review from a team as code owners December 7, 2024 00:52
@sfc-gh-lmukhopadhyay
add interop doc to toctree
747964e 
Signed-off-by: Labanya Mukhopadhyay <labanya.mukhopadhyay@snowflake.com>
@sfc-gh-lmukhopadhyay
Merge branch 'main' into lmukhopadhyay-SNOW-1805842-add-plotly-integ-…
5f6abc8 
…tests
sfc-gh-mvashishtha
sfc-gh-mvashishtha reviewed Dec 11, 2024
View reviewed changes
Copy link
Contributor

@sfc-gh-mvashishtha sfc-gh-mvashishtha left a comment

Choose a reason for hiding this comment

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

@sfc-gh-lmukhopadhyay this looks good overall! I'm leaving lots of minor comments, including several suggestions that you can add in one batch.

docs/source/modin/interoperability.rst Outdated Show resolved Hide resolved
docs/source/modin/interoperability.rst Outdated Show resolved Hide resolved
docs/source/modin/interoperability.rst Outdated Show resolved Hide resolved
docs/source/modin/interoperability.rst Outdated Show resolved Hide resolved
docs/source/modin/interoperability.rst Show resolved Hide resolved
tests/integ/modin/interoperability/plotly/test_plotly.py Outdated Show resolved Hide resolved
tests/integ/modin/interoperability/plotly/test_plotly.py Outdated Show resolved Hide resolved
tests/integ/modin/interoperability/plotly/test_plotly.py Outdated Show resolved Hide resolved
tests/integ/modin/interoperability/plotly/test_plotly.py Outdated Show resolved Hide resolved
tests/integ/modin/interoperability/plotly/test_plotly.py Outdated Show resolved Hide resolved
sfc-gh-lmukhopadhyay and others added 6 commits December 12, 2024 10:42
@sfc-gh-lmukhopadhyay @sfc-gh-mvashishtha @sfc-gh-helmeleegy
Apply doc change suggestions from code review
c176fd2 
Co-authored-by: Mahesh Vashishtha <mahesh.vashishtha@snowflake.com>
Co-authored-by: Hazem Elmeleegy <hazem.elmeleegy@snowflake.com>
@sfc-gh-lmukhopadhyay
cleanup doc and test file
fde1806 
Signed-off-by: Labanya Mukhopadhyay <labanya.mukhopadhyay@snowflake.com>
@sfc-gh-lmukhopadhyay
update doc and test changes from review
ee54d7b 
Signed-off-by: Labanya Mukhopadhyay <labanya.mukhopadhyay@snowflake.com>
@sfc-gh-lmukhopadhyay
Merge branch 'main' into lmukhopadhyay-SNOW-1805842-add-plotly-integ-…
9d4b988 
…tests
@sfc-gh-lmukhopadhyay
fix doc title
85d31a5 
Signed-off-by: Labanya Mukhopadhyay <labanya.mukhopadhyay@snowflake.com>
@sfc-gh-lmukhopadhyay
limit plotly version
4c71489 
Signed-off-by: Labanya Mukhopadhyay <labanya.mukhopadhyay@snowflake.com>
sfc-gh-mvashishtha
sfc-gh-mvashishtha reviewed Dec 13, 2024
View reviewed changes
Copy link
Contributor

@sfc-gh-mvashishtha sfc-gh-mvashishtha left a comment

Choose a reason for hiding this comment

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

nice work! I have some minor comments

docs/source/modin/interoperability.rst Show resolved Hide resolved
setup.py Show resolved Hide resolved
tests/integ/modin/interoperability/plotly/test_plotly.py Show resolved Hide resolved
tests/integ/modin/interoperability/plotly/test_plotly.py Outdated Show resolved Hide resolved
@sfc-gh-lmukhopadhyay
review changes
f56ddc5 
Signed-off-by: Labanya Mukhopadhyay <labanya.mukhopadhyay@snowflake.com>
@sfc-gh-lmukhopadhyay
Merge branch 'main' into lmukhopadhyay-SNOW-1805842-add-plotly-integ-…
c86f59f 
…tests
@sfc-gh-lmukhopadhyay
change setup comment
68fb6a5 
Signed-off-by: Labanya Mukhopadhyay <labanya.mukhopadhyay@snowflake.com>
sfc-gh-mvashishtha
sfc-gh-mvashishtha reviewed Dec 13, 2024
View reviewed changes
setup.py Outdated Show resolved Hide resolved
@sfc-gh-lmukhopadhyay
update setup comment
0dcd3e4 
Signed-off-by: Labanya Mukhopadhyay <labanya.mukhopadhyay@snowflake.com>
@sfc-gh-lmukhopadhyay
Merge branch 'main' into lmukhopadhyay-SNOW-1805842-add-plotly-integ-…
25172d3 
…tests
sfc-gh-helmeleegy
sfc-gh-helmeleegy approved these changes Dec 14, 2024
View reviewed changes
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.

LGTM, thanks Labanya!

@sfc-gh-lmukhopadhyay sfc-gh-lmukhopadhyay merged commit d6653f5 into main Dec 14, 2024
40 checks passed
@sfc-gh-lmukhopadhyay sfc-gh-lmukhopadhyay deleted the lmukhopadhyay-SNOW-1805842-add-plotly-integ-tests branch December 14, 2024 08:54
@github-actions github-actions bot locked and limited conversation to collaborators Dec 14, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@sfc-gh-mvashishtha sfc-gh-mvashishtha sfc-gh-mvashishtha approved these changes

@sfc-gh-helmeleegy sfc-gh-helmeleegy sfc-gh-helmeleegy approved these changes

@sfc-gh-nkrishna sfc-gh-nkrishna Awaiting requested review from sfc-gh-nkrishna sfc-gh-nkrishna is a code owner automatically assigned from snowflakedb/snowpark-pandas-api

@sfc-gh-aling sfc-gh-aling Awaiting requested review from sfc-gh-aling sfc-gh-aling is a code owner automatically assigned from snowflakedb/snowpark-python-api-reviewers

@sfc-gh-yuwang sfc-gh-yuwang Awaiting requested review from sfc-gh-yuwang sfc-gh-yuwang is a code owner automatically assigned from snowflakedb/snowpark-python-api-reviewers

@sfc-gh-jrose sfc-gh-jrose Awaiting requested review from sfc-gh-jrose sfc-gh-jrose is a code owner automatically assigned from snowflakedb/snowpark-python-api-reviewers

Assignees
No one assigned
Labels
NO-CHANGELOG-UPDATES This pull request does not need to update CHANGELOG.md snowpark-pandas
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@sfc-gh-lmukhopadhyay @sfc-gh-mvashishtha @sfc-gh-helmeleegy