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

Django classes plugin #1401

Closed
ANtlord opened this issue Sep 15, 2019 · 18 comments
Closed

Django classes plugin #1401

ANtlord opened this issue Sep 15, 2019 · 18 comments

Comments

@ANtlord
Copy link
Contributor

ANtlord commented Sep 15, 2019

Hello! Regarding this comment I create new issue.

So... what is the object a class is passed of as argument cls of the function?

I though it's a class of that object which the completions for. I test the code

import jedi
source = '''
from django.db import models

class Product(models.Model):
    product_name = models.CharField()
p1 = Product()
p1_name = p1.product_name
p1_name.'''
script = jedi.Script(source, 8, len('p1_name.'), 'example.py')
completions = script.completions()

so argument cls is ModelBase and that is true for p1 but it is looking for completion for p1_name. I understand that the object is clone of p1.product_name but I don't understand the relation that Jedi sees. Would you mind to explain this?

@davidhalter
Copy link
Owner

Not sure if I understand you correctly, please let me know if I misunderstood you.

The cs is <ClassValue: <Class: Product@4-6>> and metaclasses is S{<ClassValue: <Class: ModelBase@67-378>>}. This is exactly what you want. The way type inference works in Jedi is this way:

  1. p1_name. -> Find p1_name=...
  2. Found p1_name = p1.product_name
  3. Find p1
  4. Found p1 = Product()
  5. Find Product
  6. Found class Product
  7. Execute Product, because of p1 = Product() -> Returns an instance of Product.
  8. Search product_name on that instance, because of the second last line.
  9. Find completions of the returned object of (8)

Now what you need to do is provide the proper filters so that when product_name is looked at in (8) gets the return value of a string and not CharField.

Does that help?

Filters BTW are just there to find the right attribute on an instance/module/class/object.

@ANtlord
Copy link
Contributor Author

ANtlord commented Sep 17, 2019

That helps. Sorry for my "logical" sequence in that question. It's miracle that you got me.

But I've got new question. How can I create a filter of str, int, list etc.?

@davidhalter
Copy link
Owner

Please have a look at the EnumMeta case in get_metaclass_filters. There a DictFilter is created. To create a DictFilter with e.g. an attribute foo that is an int, you could for example do this:

DictFilter({'foo': cls.evaluator.builtins_module.py__getattribute__('int')})

Also note that if you have issues with the CI because of Python 2 or even anything else, I will fix it. Don't worry about that, just get it passing for a single Python version.

@ANtlord
Copy link
Contributor Author

ANtlord commented Sep 19, 2019

Ok, I've prepared mapping of basic types
https://github.com/ANtlord/jedi/blob/master/jedi/plugins/stdlib.py#L784
Could I make filters for classes from standard library. Like decimal.Decimal, datetime.timedelta, datetime.datetime etc? Is there something similar to filter building of builtin types that was described in the previous message?

UPD:
It stopped working after merging you master branch. I need update the code.

@davidhalter
Copy link
Owner

It's generally possible to use inference_state.import_module('decimal').py__getattribute__('Decimal').

It stopped working after merging you master branch. I need update the code.

Yeah sorry, I changed a ton of code.

@ANtlord
Copy link
Contributor Author

ANtlord commented Sep 20, 2019

Yeah sorry, I changed a ton of code.

Don't apologize! You, sir, make a python developer life better

I'll fix the code ASAP

@ANtlord
Copy link
Contributor Author

ANtlord commented Oct 21, 2019

I'll fix the code ASAP

One month later (facepalm)

Actually I have no idea what's wrong. I try this code

import jedi
source = '''
from enum import Enum

class Foo(Enum):
    one = 1
    two = 2

f = Foo.one
f.'''
script = jedi.Script(source, 9, len('f.'), 'example.py')
script.completions()

calling completions() causes error

TypeError: is_definition() got an unexpected keyword argument 'include_setitem'

@davidhalter
Copy link
Owner

This looks like you have to upgrade parso to the latest git master branch. Sorry for the complications.

@ANtlord
Copy link
Contributor Author

ANtlord commented Oct 28, 2019

What can I do to proceed?

@davidhalter
Copy link
Owner

I think pip uninstall parso; pip install -e git+https://github.com/davidhalter/parso.git should do the job.

@ANtlord
Copy link
Contributor Author

ANtlord commented Dec 6, 2019

Ok, I've added some improvements just now and faced a problem that is related to ForeignKey field. It is a generic field which type depends on its arguments. There are two ways to define such field:

  • catalog = ForeignKey('Catalog'). A related model class is designated by a string value
  • catalog = ForeignKey(Catalog). A related model class is designated by a traditional class

Let's resolve the second case.
How can I get the argument?
I use cls.inference_state.import_module(('decimal',)).py__getattribute__('Decimal') in order to resolve type of Decimal. How can I resolve the class I get from the argument?

@davidhalter
Copy link
Owner

davidhalter commented Dec 8, 2019

Not exactly sure what you want. Do you want to infer Catalog? Also what do you have there? Is it a Name? Because if it is you might just use name.infer().

@davidhalter
Copy link
Owner

davidhalter commented Dec 31, 2019

It might be worth creating a pull request that's in progress. I could help you there.

@ANtlord
Copy link
Contributor Author

ANtlord commented Jan 1, 2020

Sorry for such lags. I have no chance to avoid the lack of time

Do you want to infer Catalog?

Yes, I do.

Also what do you have there?

Look at the code in the PR I created just now please. Hope the code make it more clear

@ANtlord
Copy link
Contributor Author

ANtlord commented Jan 16, 2020

I've update the PR but I got new questions about the library API. The first, is there any case when lazy_values infer several values? The second, what to do if several values are inferred? How to add them into the DictFilter? I don't understand what keys should be against for the values.

@davidhalter
Copy link
Owner

Feel free to ask these questions in the pull request in the future. It's a bit easier to answer there, because I have context there :).

is there any case when lazy_values infer several values?

Theoretically, yes. Won't happen typically, but will happen in very few cases (if people do crazy stuff).

The second, what to do if several values are inferred?

IMO what you do is fine for now. You have a TODO in there as well, which is good. We know that it's not perfect. But if we cover 95% of all cases, people will be very happy. So no need to handle those cases. People will probably also not notice, because the different values will probably be very similar.

Theoretically you could work with names that return multiple values upon inferring them, but it's not really necessary here, so just ignore that.

@davidhalter
Copy link
Owner

Since you wrote a Django classes plugin now, I'm going to close this one :).

I'm still considering adding the stubs from https://github.com/typeddjango/django-stubs/tree/5b3088a17a76a77cb56a9c2241c2ddc6ccc8153a, but that's a separate issue.

Django support should be coming in the next release. :)

@davidhalter
Copy link
Owner

I made some adjustments and then added Django stubs and made sure to implement objects properly. So now Model.objects.filter().get() should return the right type. Also in general queries should be fine.

Two things are not implemented at the moment that would be nice to have:

  • Reverse model relationships (Might be slow & I'm too lazy)
  • values_list/values type returns (I'm too lazy, feel free to implement, however it's not that easy)

So I think this should be good enough for Django for now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants