Skip to content
New issue

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

Sign up for GitHub

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

Already on GitHub? Sign in to your account

Jump to bottom

Small bugs in SimpleRegressionResults #2375

Closed
qmac opened this issue Nov 22, 2017 · 5 comments · Fixed by #4071 · May be fixed by nilearn/nistats#174
Closed

Small bugs in SimpleRegressionResults #2375

qmac opened this issue Nov 22, 2017 · 5 comments · Fixed by #4071 · May be fixed by nilearn/nistats#174
Assignees
@bthirion @Remi-Gau
Labels
Bug for bug reports GLM Issues/PRs related to the nilearn.glm module.
Milestone
release 0.10.3

Comments

@qmac
Copy link

qmac commented Nov 22, 2017

(1) https://github.com/nistats/nistats/blob/master/nistats/regression.py#L366
predicted should be a method call instead of an instance variable reference

Traceback (most recent call last):
  File "nistats_model_fitting.py", line 22, in compute_rsquared
    print(model.results_[0][0.0].resid(Y))
  File "/.../nistats/nistats/regression.py", line 366, in resid
    return Y - self.predicted
TypeError: unsupported operand type(s) for -: 'float' and 'method'

(2) https://github.com/nistats/nistats/blob/master/nistats/regression.py#L392
model is not defined in SimpleRegressionResults. It is defined in the parent class LikelihoodModelResults but the initializer for the parent is never called.

Traceback (most recent call last):
  File "nistats_model_fitting.py", line 22, in compute_rsquared
    print(model.results_[0][0.0].predicted())
  File "/.../nistats/nistats/regression.py", line 392, in predicted
    X = self.model.design
AttributeError: 'SimpleRegressionResults' object has no attribute 'model'
@bthirion
Copy link
Member

Thx for pointing these issues. These classes were inherited from past projects and are actually not used correctly.
Please issue a PR if you wish to fix this.
In the longer term, I'd like to simplify that part of the code base (having less generic objects).

@bthirion
Copy link
Member

bthirion commented Jan 3, 2018

Actually, all these excellent why we should be careful with inheritance and avoid over-designing the code.

@Gilles86
Copy link
Contributor

I think I fixed it. Please give me some feedback.

https://github.com/Gilles86/nistats/tree/fix_firstlevel

@KamalakerDadi
Copy link
Contributor

@Gilles86 I think the best way is to open a PR saying that it fixes this issue. In that way you would hopefully get feedback.
Thanks

@bthirion
Copy link
Member

Indeed. let us jknow if you need help or explanations.

@kchawla-pi kchawla-pi self-assigned this Dec 6, 2018
@kchawla-pi kchawla-pi transferred this issue from nilearn/nistats Apr 2, 2020
@kchawla-pi kchawla-pi added the GLM Issues/PRs related to the nilearn.glm module. label Apr 2, 2020
@bthirion bthirion assigned bthirion and unassigned kchawla-pi Feb 10, 2021
@Remi-Gau Remi-Gau added the Bug for bug reports label Oct 19, 2023
@Remi-Gau Remi-Gau mentioned this issue Oct 19, 2023
Merged
@Remi-Gau Remi-Gau moved this to In Progress in Nilearn Generic Oct 19, 2023
@Remi-Gau Remi-Gau added this to the release 0.11.0 milestone Oct 19, 2023
@Remi-Gau Remi-Gau self-assigned this Dec 13, 2023
@github-project-automation github-project-automation bot moved this from In Progress to Done in Nilearn Generic Dec 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@bthirion bthirion

@Remi-Gau Remi-Gau

Labels
Bug for bug reports GLM Issues/PRs related to the nilearn.glm module.
Projects
Nilearn Generic
Status: Done
Milestone
release 0.10.3
Development

Successfully merging a pull request may close this issue.

[FIX] fix SimpleRegressionResults Remi-Gau/nilearn
Fix SimpleRegressionResults Gilles86/nistats
6 participants
@bthirion @Gilles86 @qmac @Remi-Gau @kchawla-pi @KamalakerDadi