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

Activity-chart-outliers #6138

Merged
merged 5 commits into from
Jun 26, 2023
Merged

Activity-chart-outliers #6138

merged 5 commits into from
Jun 26, 2023

Conversation

daiagi
Copy link
Contributor

@daiagi daiagi commented May 31, 2023

Thank you for your contribution to the KodaDot - One Stop Shop for Polkadot NFTs.

👇 __ Let's make a quick check before the contribution.

PR Type

  • Bugfix
  • Feature
  • Refactoring

Context

Did your issue had any of the "$" label on it?

$

Copilot Summary

🤖 Generated by Copilot at 87c592d

Improved the collection activity price chart by filtering out outlier data points using a new removeOutliers function. Added median and medianAbsoluteDeviation functions to @/utils/math for general math utilities.

🤖 Generated by Copilot at 87c592d

We'll filter out the outliers from the data points
With median and medianAbsoluteDeviation
We'll make the price chart more reliable and fair
Heave away, me hearties, heave away

Screenshots

Before

image

After

image

@daiagi daiagi requested a review from a team as a code owner May 31, 2023 07:44
@daiagi daiagi requested review from roiLeo and Jarsen136 and removed request for a team May 31, 2023 07:44
@kodabot
Copy link
Collaborator

kodabot commented May 31, 2023

SUCCESS @daiagi PR for issue #6109 which is assigned to you. Please wait for review and don't hesitate to grab another issue in the meantime!

@netlify
Copy link

netlify bot commented May 31, 2023

Deploy Preview for koda-canary ready!

Name Link
🔨 Latest commit 6ac499b
🔍 Latest deploy log https://app.netlify.com/sites/koda-canary/deploys/6499698eb7925500081435e2
😎 Deploy Preview https://deploy-preview-6138--koda-canary.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@reviewpad
Copy link
Contributor

reviewpad bot commented May 31, 2023

AI-Generated Summary: This pull request consists of two patches:

  1. The first patch adds two new math utility functions in utils/math.ts file:

    • median() function calculates the median of an array of numbers.
    • mediaAbsoluteDeviation() function calculates the median absolute deviation (MAD) of an array of numbers.
  2. The second patch makes changes to ActivityChart.vue and activity/utils.ts files:

    • It imports the removeOutliers function and uses this function to filter outlier data points from the list events.
    • In activity/utils.ts, a new removeOutliers() function is added. This function filters out data points with a deviation above the specified threshold from the median.

@reviewpad reviewpad bot added small Pull request is small waiting-for-review labels May 31, 2023
Copy link
Contributor

@roiLeo roiLeo left a comment

Choose a reason for hiding this comment

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

@daiagi
Copy link
Contributor Author

daiagi commented Jun 1, 2023

Tested on https://deploy-preview-6138--koda-canary.netlify.app/stmn/collection/2022/activity

Doesn't load for me compared to beta

hi @roiLeo
I've created this PR:

this collection is very big and the issue goes back to these:

Copy link
Contributor

@roiLeo roiLeo left a comment

Choose a reason for hiding this comment

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

hi @roiLeo I've created this PR:

* [Test/activity-chart-outliers #6146](https://github.com/kodadot/nft-gallery/pull/6146)
  to illustrate a point, in that PR there are no code diff compare to main yet that collection doesn't load
  https://deploy-preview-6146--koda-canary.netlify.app/stmn/collection/2022/activity

Alright noted! personally I would have hidden this feature (Activity Tab) for the time being until fixed #5540 (comment)

@prury
Copy link
Member

prury commented Jun 13, 2023

About the outliers, they are still present when i open this deployed version:
https://deploy-preview-6138--koda-canary.netlify.app/ksm/gallery/11778219-2644199cf3652aaa78-KQ01-008_PIZZA-00000008

image

@prury prury added the S-changes-requested-🤞 PR is almost good to go, just some fine tunning label Jun 13, 2023
@daiagi
Copy link
Contributor Author

daiagi commented Jun 14, 2023

About the outliers, they are still present when i open this deployed version: https://deploy-preview-6138--koda-canary.netlify.app

the issue was about collection activity chart

the chart you've mentioned is a single NFT chart and was not touched in this PR

as to the specific example you've presented, not sure why you are saying there are outliers, the graph looks well behaved and easy to read to me

@prury
Copy link
Member

prury commented Jun 14, 2023

About the outliers, they are still present when i open this deployed version: https://deploy-preview-6138--koda-canary.netlify.app

the issue was about collection activity chart

* [Show outliers on chart in activity #6109 (comment)](https://github.com/kodadot/nft-gallery/issues/6109#issue-1727133764)

the chart you've mentioned is a single NFT chart and was not touched in this PR

as to the specific example you've presented, not sure why you are saying there are outliers, the graph looks well behaved and easy to read to me

@daiagi I apologize, the whole time i was reading outlines instead of outliners and i did not knew this collection activity page existed, will verify this PR properly today, only did a quick check yesterday and assumed it wasn't working.

@prury prury removed the S-changes-requested-🤞 PR is almost good to go, just some fine tunning label Jun 14, 2023
@prury
Copy link
Member

prury commented Jun 14, 2023

@exezbcz exezbcz mentioned this pull request Jun 14, 2023
@daiagi
Copy link
Contributor Author

daiagi commented Jun 17, 2023

@prury

let's let it roll
there is no "best final solution"
in any case we will need to use 1 method to clean data from charts that may look quite different from each other
it will work better for some and worst for other

it looks better now and the data is easier to read then it was before, so let's continue
we can always tweak it as needed in the future

@prury prury added the S-works-for-me-✅ qa-guild has tested PR from end user perspective and functionality worked label Jun 17, 2023
@daiagi daiagi requested a review from preschian June 23, 2023 06:56
@daiagi
Copy link
Contributor Author

daiagi commented Jun 23, 2023

@Jarsen136 review please?

@codeclimate
Copy link

codeclimate bot commented Jun 26, 2023

Code Climate has analyzed commit 6ac499b and detected 0 issues on this pull request.

View more on Code Climate.

@vikiival
Copy link
Member

pay 35 usd

@vikiival vikiival merged commit cbe6b72 into main Jun 26, 2023
@vikiival vikiival deleted the activity-chart-outliers branch June 26, 2023 11:22
@yangwao
Copy link
Member

yangwao commented Jun 26, 2023

😍 Perfect, I’ve sent the payout
💵 $35 @ 5.2 USD/DOT ~ 6.731 $DOT
🧗 1cAsKZYNRb8dkSCpn4eVkEn6ycNZTGoDo5dGDgB8J1UUWK8
🔗 0x3cb6a99d643fcdc4d303f84c76707035e757cfcaaa7720e3bf1437857665c141

🪅 Let’s grab another issue and get rewarded!
🪄 github.com/kodadot/nft-gallery/issues

@yangwao yangwao added the paid pull-request has been paid label Jun 26, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
paid pull-request has been paid S-works-for-me-✅ qa-guild has tested PR from end user perspective and functionality worked small Pull request is small waiting-for-review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Show outliers on chart in activity
8 participants