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

Instance of 'CharField' has no 'upper' member warning (no-member) #7

Closed
alisaifee opened this issue Feb 5, 2014 · 8 comments
Closed

Comments

@alisaifee
Copy link

When I access a CharField in a model and call the upper() method, pylint complains. However, this is valid at runtime.

@jproffitt
Copy link
Contributor

This is because at run time, model fields are treated like the underlying python type. So a CharField, for example, is treated like a str.

I'd be happy to work on a solution to this. But I'm not sure what the best approach is. I attempted to solve it here: jproffitt@42548e4. But maybe there's a better way?

@alisaifee
Copy link
Author

as far as the CharField goes your solution solves it, however eventually there will be another field that complains. I'm not too familiar with the internals of pylint but if there was some way to hint to it that the instance of a Field subclass should be treated as the same type as the return value of the to_python() instance method, that could generalise well.

@carlio
Copy link
Collaborator

carlio commented Jun 19, 2014

@jproffitt, actually I think your solution is very very elegant! That's what the Django metaprogramming does anyway, at runtime, the attributes on the model class are str or int or whatever. All of the validation stuff is moved into a _meta class attribute. So 👍 from me.

@alisaifee You're right a more general solution would be nice, but I don't know if pylint's inference is powerful enough to figure out the value of to_python(), but even if it is, I don't know of a way to "connect" up the values. But since @jproffitt's solution is essentially the same as what Django does, I think it's the best way to go.

ForeignKey will still be difficult though!

@jproffitt
Copy link
Contributor

I can't think of any way to do a more general solution. I don't think pylint can do that... if it can, let me know, because that would be awesome.

I will create a PR with my solution. Is there any other fields I should add besides DateTimeField, DateField, and CharField? IntegerField?

I don't have any ideas for ForeignKey.

carlio added a commit that referenced this issue Sep 14, 2014
…go fields based on the idea of @jproffitt - they all now extend both the Django field (for constructor use and direct manipulation and usage) as well as a type which accurately reflects what they are transformed into by the model meta-programming
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
carlio added a commit that referenced this issue Sep 14, 2014
…les using a similar technique to #7 and the solution suggested by @enoren in #12 - namely, subclass both the Django Field type and the type that it is converted to by Django meta-programming
@carlio
Copy link
Collaborator

carlio commented Sep 14, 2014

@jproffitt I took your suggestion and changed it a little, so now all Field classes extend the Django Field and the type they represent, which removes almost all of the errors 👍

This will be available in the 0.5 release (coming soon)

@jproffitt
Copy link
Contributor

What about ForeignKeys? That basically needs to be an instance of whatever model the ForeignKey is pointing to. Is there some crazy thing we can do with that? Or is it not even a problem?

@carlio
Copy link
Collaborator

carlio commented Sep 22, 2014

ForeignKeys will be probably be really hard, unfortunately. The main problem is that pylint emits problems as it finds them, and knowing about ForeignKeys would require knowing the contents of all model.py files, I think. I certainly want to try though! There are 'reporters' or something which may make it possible to emit a bunch of messages at the end or to post-process. I'll open a new issue for that though.

@carlio
Copy link
Collaborator

carlio commented Sep 22, 2014

See #21

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