Skip to content

Conversation

@mpolson64
Copy link
Contributor

Summary: p_feasible is very important to express to users and should be promoted to its own dedicated plot

Differential Revision: D83864978

@meta-cla meta-cla bot added the CLA Signed Do not delete this pull request or issue due to inactivity. label Oct 6, 2025
@codecov-commenter
Copy link

codecov-commenter commented Oct 6, 2025

Codecov Report

❌ Patch coverage is 93.33333% with 2 lines in your changes missing coverage. Please review.
⚠️ Please upload report for BASE (main@d64ce05). Learn more about missing BASE report.

Files with missing lines Patch % Lines
ax/analysis/plotly/arm_effects.py 88.88% 1 Missing ⚠️
ax/analysis/plotly/scatter.py 90.00% 1 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##             main    #4398   +/-   ##
=======================================
  Coverage        ?   96.20%           
=======================================
  Files           ?      553           
  Lines           ?    56717           
  Branches        ?        0           
=======================================
  Hits            ?    54564           
  Misses          ?     2153           
  Partials        ?        0           

☔ 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.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@meta-codesync
Copy link

meta-codesync bot commented Oct 6, 2025

@mpolson64 has exported this pull request. If you are a Meta employee, you can view the originating Diff in D83864978.

mpolson64 added a commit to mpolson64/Ax that referenced this pull request Oct 6, 2025
Summary:

p_feasible is very important to express to users and should be promoted to its own dedicated plot. Including it in ArmEffects increases visual density past a useful point, and add significant code complexity.

Differential Revision: D83864978
mpolson64 added a commit to mpolson64/Ax that referenced this pull request Oct 6, 2025
Summary:

p_feasible is very important to express to users and should be promoted to its own dedicated plot. Including it in ArmEffects increases visual density past a useful point, and add significant code complexity.

Differential Revision: D83864978
mpolson64 added a commit to mpolson64/Ax that referenced this pull request Oct 7, 2025
Summary:

p_feasible is very important to express to users and should be promoted to its own dedicated plot. Including it in ArmEffects increases visual density past a useful point, and add significant code complexity.

Reviewed By: lena-kashtelyan

Differential Revision: D83864978
mpolson64 added a commit to mpolson64/Ax that referenced this pull request Oct 9, 2025
Summary:

p_feasible is very important to express to users and should be promoted to its own dedicated plot. Including it in ArmEffects increases visual density past a useful point, and add significant code complexity.

Reviewed By: lena-kashtelyan

Differential Revision: D83864978
mpolson64 added a commit to mpolson64/Ax that referenced this pull request Oct 9, 2025
Summary:

p_feasible is very important to express to users and should be promoted to its own dedicated plot. Including it in ArmEffects increases visual density past a useful point, and add significant code complexity.

Reviewed By: lena-kashtelyan

Differential Revision: D83864978
mpolson64 added a commit to mpolson64/Ax that referenced this pull request Oct 9, 2025
Summary:

p_feasible is very important to express to users and should be promoted to its own dedicated plot. Including it in ArmEffects increases visual density past a useful point, and add significant code complexity.

Reviewed By: lena-kashtelyan

Differential Revision: D83864978
mpolson64 added a commit to mpolson64/Ax that referenced this pull request Oct 9, 2025
Summary:

p_feasible is very important to express to users and should be promoted to its own dedicated plot. Including it in ArmEffects increases visual density past a useful point, and add significant code complexity.

Reviewed By: lena-kashtelyan

Differential Revision: D83864978
mpolson64 added a commit to mpolson64/Ax that referenced this pull request Oct 14, 2025
Summary:

p_feasible is very important to express to users and should be promoted to its own dedicated plot. Including it in ArmEffects increases visual density past a useful point, and add significant code complexity.

Reviewed By: lena-kashtelyan

Differential Revision: D83864978
mpolson64 added a commit to mpolson64/Ax that referenced this pull request Oct 14, 2025
Summary:

p_feasible is very important to express to users and should be promoted to its own dedicated plot. Including it in ArmEffects increases visual density past a useful point, and add significant code complexity.

Reviewed By: lena-kashtelyan

Differential Revision: D83864978
mpolson64 added a commit to mpolson64/Ax that referenced this pull request Oct 14, 2025
Summary:

p_feasible is very important to express to users and should be promoted to its own dedicated plot. Including it in ArmEffects increases visual density past a useful point, and add significant code complexity.

Reviewed By: lena-kashtelyan

Differential Revision: D83864978
mpolson64 added a commit to mpolson64/Ax that referenced this pull request Oct 14, 2025
Summary:

p_feasible is very important to express to users and should be promoted to its own dedicated plot. Including it in ArmEffects increases visual density past a useful point, and add significant code complexity.

Reviewed By: lena-kashtelyan

Differential Revision: D83864978
… logic (facebook#4396)

Summary:

Was hardcoded with custom logic before, now we just use the attr on these two Analyses directly. This shaves down custom handling and allows us to override easily when necessary.

Reviewed By: lena-kashtelyan

Differential Revision: D83869841
Summary:

As discussed in chat. This is currently not used, adds complexity, and should be broken out into its own "progression" plot in the future

Reviewed By: lena-kashtelyan

Differential Revision: D83863709
Summary:

p_feasible is very important to express to users and should be promoted to its own dedicated plot. Including it in ArmEffects increases visual density past a useful point, and add significant code complexity.

Reviewed By: lena-kashtelyan

Differential Revision: D83864978
mpolson64 added a commit to mpolson64/Ax that referenced this pull request Oct 15, 2025
Summary:

p_feasible is very important to express to users and should be promoted to its own dedicated plot. Including it in ArmEffects increases visual density past a useful point, and add significant code complexity.

Reviewed By: lena-kashtelyan

Differential Revision: D83864978
mpolson64 added a commit to mpolson64/Ax that referenced this pull request Oct 15, 2025
Summary:

p_feasible is very important to express to users and should be promoted to its own dedicated plot. Including it in ArmEffects increases visual density past a useful point, and add significant code complexity.

Reviewed By: lena-kashtelyan

Differential Revision: D83864978
@meta-codesync meta-codesync bot closed this in b57d277 Oct 16, 2025
@meta-codesync
Copy link

meta-codesync bot commented Oct 16, 2025

This pull request has been merged in b57d277.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA Signed Do not delete this pull request or issue due to inactivity. fb-exported Merged meta-exported

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants