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

[SEDONA-558] Fix and improve SedonaPyDeck behavior #1415

Merged
merged 6 commits into from
May 18, 2024

Conversation

furqaankhan
Copy link
Contributor

@furqaankhan furqaankhan commented May 17, 2024

Did you read the Contributor Guide?

Is this PR related to a JIRA ticket?

Description

Current implementation

  • Adds the optional parameters map_style, and map_provider as data members to the pdk map object.

    eg: map_p.map_style = map_style

  • The current behavior of PyDeck doesn't update the map object with the additional configuration added via data members.

Behavior of PyDeck

  • The default map_style is 'dark' and map_provider is 'carto'. If you configure map_style to 'satellite' it will not change the map_provider to Mapbox or Google Maps API.

What changes were proposed in this PR?

  • Expose api_key parameter of PyDeck
  • Create a new method to create the PyDeck object to handle the default behavior of PyDeck.
  • Default to Mapbox map provider if the map_style is set to 'satellite' and if map_provider is Carto (the default).

How was this patch tested?

  • Passed existing tests.

Did this PR include necessary documentation updates?

  • Yes, I have updated the documentation.

Copy link
Member

@jiayuasu jiayuasu left a comment

Choose a reason for hiding this comment

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

Why is this PR changing these files?

Copy link
Member

@jiayuasu jiayuasu left a comment

Choose a reason for hiding this comment

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

Is it possible to add some test cases?

Copy link
Member

Choose a reason for hiding this comment

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

Why is this PR changing these files?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

They are phony changes, these changes have been merged before.

@furqaankhan
Copy link
Contributor Author

To test if the API key has been added successfully is not possible. However, I can add tests for other behavior changes.

@jiayuasu jiayuasu added this to the sedona-1.6.1 milestone May 18, 2024
@jiayuasu jiayuasu merged commit 30308e6 into apache:master May 18, 2024
28 checks passed
@furqaankhan furqaankhan deleted the fix-sedona-pydeck branch July 16, 2024 01:39
Kontinuation pushed a commit to Kontinuation/sedona that referenced this pull request Oct 11, 2024
* fix: python dataframe API constructor Function registrations

* fix: SedonaPyDeck behavior as per expectation

* docs: update docs

* docs: update docs 2

* chore: add test for SedonaPyDeck

* chore: add test for SedonaPyDeck 2
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants