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

Plotting Widgets #1505

Merged
merged 11 commits into from Sep 29, 2022
Merged

Plotting Widgets #1505

merged 11 commits into from Sep 29, 2022

Conversation

ChakriCherukuri
Copy link
Member

Added plotting widgets module which contains compound plotting widgets which subclass Figure

@ibdafna
Copy link
Member

ibdafna commented Jul 17, 2022

Will review this week.

@ibdafna
Copy link
Member

ibdafna commented Jul 21, 2022

@ChakriCherukuri issue seems to be upstream with traittypes:

https://github.com/jupyter-widgets/traittypes/blob/af2ebeec9e58b73a12d4cf841bd506d6eadb8868/traittypes/traittypes.py#L185

The dType is defined there in the constructor.

@ljcno3
Copy link

ljcno3 commented Jul 21, 2022

Please unsubscribe me from this Thx

@ibdafna
Copy link
Member

ibdafna commented Jul 21, 2022

Please unsubscribe me from this Thx

Hi there, no one subscribed you - only you can subscribe yourself to updates. Check if you're following the repo or something like that. There's nothing we can do for you on our end. Thanks!

@ljcno3
Copy link

ljcno3 commented Jul 21, 2022 via email

@ChakriCherukuri
Copy link
Member Author

@martinRenou can you please advise how to fix the traittypes warnings? thx

@martinRenou
Copy link
Member

All I can do I guess is delete my GitHub account 🙁

This sounds a bit radical ahah. You have an "unsubscribe" link in the email you received, maybe clicking it could help: https://github.com/notifications/unsubscribe-auth/ADCNFJORYFEFETBAP3742E3VVGJLPANCNFSM52XMT7BQ

@ChakriCherukuri Sure I'll try to find time to have a look

@ibdafna
Copy link
Member

ibdafna commented Jul 27, 2022 via email

@ChakriCherukuri
Copy link
Member Author

@chakri I pointed out where the issue is above. We'll need an upstream PR to traittypes

On Wed, Jul 27, 2022, 16:35 martinRenou @.> wrote: All I can do I guess is delete my GitHub account 🙁 This sounds a bit radical ahah. You have an "unsubscribe" link in the email you received, maybe clicking it could help: https://github.com/notifications/unsubscribe-auth/ADCNFJORYFEFETBAP3742E3VVGJLPANCNFSM52XMT7BQ @ChakriCherukuri https://github.com/ChakriCherukuri Sure I'll try to find time to have a look — Reply to this email directly, view it on GitHub <#1505 (comment)>, or unsubscribe https://github.com/notifications/unsubscribe-auth/AFZICWPZON33GW2H6XOPWV3VWFCLLANCNFSM52XMT7BQ . You are receiving this because your review was requested.Message ID: @.>

So @ibdafna can you merge it then?

Comment on lines +119 to +123
t = np.linspace(0, 2 * np.pi, 1000)
band_data_x, band_data_y = (
self.scaled_band_data * np.cos(t),
self.scaled_band_data * np.sin(t),
)
Copy link
Member

Choose a reason for hiding this comment

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

It would be nice to have a Circle primitive (similar to the Lines one), so we don't need to create a circle based on a thousand segments.

This would be more efficient as it would mean sending 3 float values (center + radius) instead of 2000 float values to the front-end.

Just adding this as a note, I don't think we should block this PR waiting for such a primitive.

@ChakriCherukuri
Copy link
Member Author

@ibdafna @martinRenou can we please merge this if we are OK with the warnings?

@ChakriCherukuri
Copy link
Member Author

@martinRenou I reverted the code to use an empty DataFrame constructor. Can you please review? Thx

@ChakriCherukuri
Copy link
Member Author

@martinRenou can you please merge if this looks good?

@martinRenou
Copy link
Member

Thanks! We should get the CI green before merging

@martinRenou martinRenou merged commit 4a5b341 into bqplot:master Sep 29, 2022
@martinRenou
Copy link
Member

meeseeksdev please backport to 0.12.x

meeseeksmachine pushed a commit to meeseeksmachine/bqplot that referenced this pull request Sep 29, 2022
martinRenou added a commit that referenced this pull request Sep 29, 2022
…5-on-0.12.x

Backport PR #1505 on branch 0.12.x (Plotting Widgets)
maartenbreddels pushed a commit to maartenbreddels/bqplot that referenced this pull request Feb 24, 2023
maartenbreddels pushed a commit to maartenbreddels/bqplot that referenced this pull request Mar 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants