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.

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

False positives (especially on forms.py) #11

Closed
chrisma opened this issue Jun 12, 2014 · 10 comments
Closed

False positives (especially on forms.py) #11

chrisma opened this issue Jun 12, 2014 · 10 comments

Comments

@chrisma
Copy link

chrisma commented Jun 12, 2014

We tried using landscape.io with this plugin.
Unfortunately, there seem to be false positives (especially concerning forms).
See for example here: https://landscape.io/github/hubx/EvaP/4/modules/evap/fsr/forms.py

@carlio
Copy link
Collaborator

carlio commented Jun 16, 2014

Hi @chrisma, thanks for the bug report. This was two issues:

@chrisma
Copy link
Author

chrisma commented Jun 17, 2014

Hey,
thanks for the quick fix. Looks good!
pylint-django dealing with form field constructors would be awesome.

carlio added a commit that referenced this issue Sep 14, 2014
…form fields should now have the behaviour both of the parent Django Field type as well as the more fundamental type they represent
@karyon
Copy link

karyon commented Sep 25, 2014

i don't know exactly what the problem with the form field constructors was, but looking at our current code, it seems to be fixed in landscape:

https://landscape.io/github/fsr-itse/EvaP/130/modules/evap/fsr/forms.py

there is one error left in that file. i don't know whether that's an actual error in the code or another landscape fail.

@carlio
Copy link
Collaborator

carlio commented Oct 6, 2014

I think that last remaining error is not Landscape but rather Pylint. It seems like it's warning because the class passed to super is not the direct parent, but rather the grandparent. I don't know if this is a bug or a warning, but it's in pylint proper rather than this plugin so I'll close this issue.

@carlio carlio closed this as completed Oct 6, 2014
@karyon
Copy link

karyon commented Nov 3, 2014

it seems this bug came back, see https://landscape.io/github/fsr-itse/EvaP/146/modules/evap/fsr/forms.py

@karyon
Copy link

karyon commented Nov 10, 2014

should i file a new bug for this?

@carlio
Copy link
Collaborator

carlio commented Nov 10, 2014

Hi @karyon - it's still not fixed, because it's still a problem in pylint/astroid - see https://bitbucket.org/logilab/pylint/issue/24/e1002-message-for-djangoformsmodelform . Unfortunately it's proving to be a difficult thing to fix! However it's upstream from pylint-django so please leave this issue closed.

@karyon
Copy link

karyon commented Nov 10, 2014

i'm sorry if i was unclear, but by "this bug came back" i meant that all the false positives in our forms came back. please have a look at https://landscape.io/github/fsr-itse/EvaP/146/modules/evap/fsr/forms.py, my problem should be immediately clear.

@carlio
Copy link
Collaborator

carlio commented Nov 10, 2014

@karyon Sorry I misunderstood.

I investigated further and it seems like the problem is because Django was not installed correctly in the Landscape checking environment. The score went from 90% to 63% from check 138 to check 143 and if you look on the requirements for 138 and the requirements for 143 you can see that Django stopped installing correctly then.

I don't know why this is, I will look into it. This is a Landscape bug however, so I have opened a new issue here: landscapeio/landscape-issues#88

@karyon
Copy link

karyon commented Nov 10, 2014

thanks for looking into this!

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

No branches or pull requests

3 participants