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

Format Data from Domain #1827

Merged
merged 39 commits into from
May 4, 2021
Merged

Format Data from Domain #1827

merged 39 commits into from
May 4, 2021

Conversation

jhumbug
Copy link
Contributor

@jhumbug jhumbug commented Apr 23, 2021

Adds a format data function in global data util to keep data points from reaching out of their supplied domains and to remove points that are entirely outside of the domain.

@jhumbug
Copy link
Contributor Author

jhumbug commented Apr 23, 2021

note: still need to refactor the function to be less verbose

@jhumbug
Copy link
Contributor Author

jhumbug commented Apr 23, 2021

open question: For the chart types with non-typical accessors (VictoryBoxPlot, VictoryCandlestick, VictoryErrorbar) should we switch their groupComponent to VictoryClipContainer in order to keep them within their Chart wrapper when setting domain?

@boygirl
Copy link
Contributor

boygirl commented Apr 26, 2021

open question: For the chart types with non-typical accessors (VictoryBoxPlot, VictoryCandlestick, VictoryErrorbar) should we switch their groupComponent to VictoryClipContainer in order to keep them within their Chart wrapper when setting domain?

I feel like if we went that route for some, it would make more sense to use a clipped group for all the components and just roll with the breaking change.

@jhumbug jhumbug marked this pull request as ready for review April 28, 2021 12:56
@jhumbug
Copy link
Contributor Author

jhumbug commented Apr 29, 2021

At this point I believe everything's accounted for. I even went through and found several issues from the past that were related to this and tested their use cases to make sure this solves it. It allowed me to find a VictoryStack issue that I addressed.

@boygirl
Copy link
Contributor

boygirl commented May 4, 2021

@jhumbug This is missing data filtering on VictoryVoronoi. That component isn't very commonly used, but should be updated for completeness. Other than that, this is looking good to go. Nice work!

@boygirl
Copy link
Contributor

boygirl commented May 4, 2021

Approved! Nice work, @jhumbug

@jhumbug
Copy link
Contributor Author

jhumbug commented May 4, 2021

Approved! Nice work, @jhumbug

yay! thank you

@jhumbug jhumbug merged commit 65bb96c into main May 4, 2021
@jhumbug jhumbug deleted the task/format-from-domain branch May 5, 2021 19:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants