Skip to content

Conversation

@esantorella
Copy link
Contributor

Summary: It makes more sense there than in ax.core.base_trial.py

Reviewed By: saitcakmak

Differential Revision: D89894244

Summary:

**Context**:

`Experiment.lookup_data_for_trial` looks up a single trial's data, and `Experiment.lookup_data` has an argument `trial_indices`. Previously, it was somewhat useful to have both becauase it was faster to look up data for a single trial than to filter that trial's data from the full DataFrame, but this is no longer the case now that data is represented as a single data frame rather than a dict of DataFrames. So `Experiment.lookup_data_for_trial` is not adding value.

**This PR**:
* Removes `Experiment.lookup_data_for_trial` and updates call sites

Reviewed By: saitcakmak

Differential Revision: D89811675
Summary: It makes more sense there than in ax.core.base_trial.py

Reviewed By: saitcakmak

Differential Revision: D89894244
@meta-codesync
Copy link

meta-codesync bot commented Jan 6, 2026

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

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

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 96.72%. Comparing base (60f5ca9) to head (d4f6e56).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #4743      +/-   ##
==========================================
- Coverage   96.73%   96.72%   -0.01%     
==========================================
  Files         582      582              
  Lines       60699    60693       -6     
==========================================
- Hits        58716    58708       -8     
- Misses       1983     1985       +2     

☔ 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 Jan 6, 2026

This pull request has been merged in aa29dd8.

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