Skip to content

Feature/result cast#989

Merged
Jammy2211 merged 81 commits intomainfrom
feature/result_cast
Apr 8, 2024
Merged

Feature/result cast#989
Jammy2211 merged 81 commits intomainfrom
feature/result_cast

Conversation

@Jammy2211
Copy link
Copy Markdown
Collaborator

New API for how the Analysis class returns a Return.

Now, the Analysis class has a Result class attribute, which a user can overwrite with their own custom Result class.

This removes a number of unecessary make_result functions that end up populating Analysis objects and is a cleaner API for users, as shown in the docs updated in this PR.

Copy link
Copy Markdown
Collaborator

@rhayes777 rhayes777 left a comment

Choose a reason for hiding this comment

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

We need to fix those interpolator tests


residual_map = self.data - model_data_1d
chi_squared_map = (residual_map / self.noise_map) ** 2.0
log_likelihood = -0.5 * sum(chi_squared_map)
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Just return this line rather than defining an ephemeral variable

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

This is for the example module, which users might read, so intentionally simple.

self._instance = instance or ModelInstance()
self._samples = samples or MockSamples(
max_log_likelihood_instance=self.instance, model=model or ModelMapper()
# max_log_likelihood_instance=self.instance,
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Commented code

Comment on lines +42 to +46
def model_absolute(self, a):
try:
return self.samples_summary.model_absolute(a)
except AttributeError:
return self.model
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Is it worth adding a warning?

Comment on lines +305 to +311
try:
Samples.from_csv(
paths=self.paths,
model=self.model,
)
except FileNotFoundError:
pass
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Did you mean to return this?

# )


# def test_interpolate(interpolator):
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Ooof why is this all commented out?

@Jammy2211 Jammy2211 merged commit f71f9cd into main Apr 8, 2024
@Jammy2211 Jammy2211 deleted the feature/result_cast branch April 18, 2024 12:44
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.

2 participants