Skip to content

Merge Estimator.summary() and Estimator.fit() #103

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Open
tsalo opened this issue Jun 3, 2022 · 1 comment · May be fixed by #112
Open

Merge Estimator.summary() and Estimator.fit() #103

tsalo opened this issue Jun 3, 2022 · 1 comment · May be fixed by #112
Labels
breaking-change PRs that change results or interfaces. enhancement New feature or request

Comments

@tsalo
Copy link
Member

tsalo commented Jun 3, 2022

In PyMARE, fit and fit_dataset both return Estimators, rather than Results. Instead, there's an additional step of calling Estimator.summary() to produce the Results object. This is different from NiMARE, where fitting an Estimator returns a MetaResult.

I don't know if the extra step is necessary. I.e., does fit returning an Estimator instead of a Result serve a purpose?

@tsalo tsalo added enhancement New feature or request breaking-change PRs that change results or interfaces. labels Jun 3, 2022
@tsalo
Copy link
Member Author

tsalo commented Jun 10, 2022

Actually, for the sake of scikit-learn styling, what if we instead merged fit and fit_dataset, and then had fit (returns fitted Estimator), fit_transform (returns Results), and transform (returns Results)?

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
breaking-change PRs that change results or interfaces. enhancement New feature or request
Projects
None yet
1 participant