Skip to content

Conversation

misspran
Copy link
Contributor

@misspran misspran commented Sep 2, 2025

I added a draft in hopes of getting some eyes on the stats code.

@misspran
Copy link
Contributor Author

hi @beatrice-acasandrei, when you get a chance will you eye my draft PR? The return object for the new stats method is still pretty big but I can remove stuff that are extraneous once we are firm on what information we want to display on the front-end.

@misspran misspran force-pushed the PCF-634-new-stats-analysis branch 3 times, most recently from 682727b to 167f5c8 Compare September 16, 2025 15:37
@misspran
Copy link
Contributor Author

@beatrice-acasandrei I added a 2nd serializer as suggested. I still need to add more tests but if you don't mind taking a glance at the changes

@misspran misspran marked this pull request as ready for review September 17, 2025 13:42
@misspran misspran removed the request for review from esanuandra September 18, 2025 19:47
@misspran
Copy link
Contributor Author

misspran commented Sep 30, 2025

@beatrice-acasandrei I think bugs should be fixed and updated. I've also grouped the return values based on your feedback and updated the serializer.

@misspran misspran force-pushed the PCF-634-new-stats-analysis branch from e39936a to e5148d0 Compare October 2, 2025 00:59

# Statistical methods for performance analysis
scipy==1.15.3
numpy==2.2.6
Copy link
Collaborator

Choose a reason for hiding this comment

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

Could we see if we can remove the numpy/scipy requirements? We already have numpy and scipy and a major version update to numpy could be problematic for other tooling.

If needed, we could also try downgrading the cliffs-delta/kdepy ones to match the versions of numpy we currently have.

urllib3==2.0.3

# Statistical methods for performance analysis
scipy==1.15.3
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think this file contains requirements needed for testing specifically. They should already be covered by the requirements/common.in file so I believe you can remove them from here.

@misspran misspran force-pushed the PCF-634-new-stats-analysis branch from bc36d7a to 40ec5eb Compare October 2, 2025 19:14
Copy link
Collaborator

@gmierz gmierz left a comment

Choose a reason for hiding this comment

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

r+, thanks for resolving the dev requirements changes!

@beatrice-acasandrei beatrice-acasandrei merged commit b626c64 into mozilla:master Oct 7, 2025
6 checks passed
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.

3 participants