Skip to content

Add function to plot scaling data.#11

Merged
micaeljtoliveira merged 1 commit intomainfrom
scaling_plots
Oct 10, 2025
Merged

Add function to plot scaling data.#11
micaeljtoliveira merged 1 commit intomainfrom
scaling_plots

Conversation

@micaeljtoliveira
Copy link
Member

No description provided.

@codecov
Copy link

codecov bot commented Oct 3, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 100.00%. Comparing base (a491eb6) to head (14fdca3).
⚠️ Report is 2 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff            @@
##              main       #11   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files            8         9    +1     
  Lines          211       281   +70     
=========================================
+ Hits           211       281   +70     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copy link
Collaborator

@manodeep manodeep left a comment

Choose a reason for hiding this comment

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

I looked through the PR and had some comments/suggestions

Do you mind adding a plot that is generated by the plotting function into the conversation - that way we will have a reference

@micaeljtoliveira
Copy link
Member Author

@manodeep Since the code was mostly written by @edoyango, I'll let him answer the comments.

@edoyango
Copy link
Collaborator

edoyango commented Oct 8, 2025

@manodeep I've addressed most of your comments in the latest commits. Can you have another look?

Copy link
Collaborator

@manodeep manodeep left a comment

Choose a reason for hiding this comment

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

@edoyango Thank you for simplifying the code :)

I made some minor suggestions about improving the docs and usability

Copy link
Member Author

@micaeljtoliveira micaeljtoliveira left a comment

Choose a reason for hiding this comment

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

@edoyango Thanks for taking care of addressing the comments and sorry for my slow response.

Now, after using the code in this PR a bit, I also have some comments and suggestions.

@micaeljtoliveira
Copy link
Member Author

@edoyango This is all good from my side, but I can't approve the PR, as I opened it 😆 Can you approve for me?

@micaeljtoliveira
Copy link
Member Author

PS: Please make sure you either clean-up the git history before merging or use the squash and merge option.

Copy link
Collaborator

@manodeep manodeep left a comment

Choose a reason for hiding this comment

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

@edoyango I made two minor comments about docstrings; otherwise looks good. I am assuming that you have checked that the code runs and results in a (reasonable-looking) plot

@edoyango
Copy link
Collaborator

edoyango commented Oct 9, 2025

@manodeep done! can you approve?

Copy link
Collaborator

@edoyango edoyango left a comment

Choose a reason for hiding this comment

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

Approving :P

@micaeljtoliveira micaeljtoliveira merged commit 51cdf8f into main Oct 10, 2025
8 checks passed
@micaeljtoliveira micaeljtoliveira deleted the scaling_plots branch October 10, 2025 00:13
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