DjangoBot: #django-dev

hey ericflo. want to talk to you about #6095

brosner: KK, I haven't had time to look at it in quite a while, but I'm ready to start looking at it again.

cool. i'd like to revisit the add/remove limitation

I was thinking about that myself, and I agree.

We gotta get Russ on here

He's the one who suggested to remove it

yeah

And the ticket's not really a good place for that kind of discussion

The ticket is actually getting kind of huge

i guess it just feels really limiting that its called a ManyToManyField and ModelForms wont work

i understand why

Me too

But we do a similar thing with update

and how one arg applies to all of the items in the qs

like my really specific (maybe not) use case was having an intermediary table where the "extra" fields were determined automatically like a sort_order.

brosner: Not sure what you mean, like they have a default?

basically. i wrote my own version of save_instance to deal with it

ugh, this is a tough one

brb 5 min :(

DjangoBot, who semente

pm the bot

sorry, my bad

brosner: OK back

brosner: The one thing that's nice about the current solution, is that once it gets into trunk, we can extend the syntax later for more syntactic sugar.

brosner: As people start to use intermediary models, there'll be more people who can think about a good syntax

brosner: That's both good and bad, I guess

well i begin to wonder if that will end up being just given intermediary model instances?

What do you mean?

if maybe there was an extra method that just took the intermediary model instances. so like Membership instances

then something like a clean method could do that in a ModelForm for instance

Extra method on what?

the many to many manager

But then you still have to query for those instances directly, meaning there's no real gain in functionality, unless I'm seeing it wrong.

i am not talking about querying though

those use cases seem to be covered

unless i am missing something

brosner: So as an example, my_group_instance.members.add(my_person_instance, Membership(person=my_person_instance, group=my_group_instance, date_joined=datetime.now(), invite_reason="She is cool."))

?

ericflo: I think, for a first cut, you're overthinking the problem... I'd be pretty happy with just creating Memberships by hand at first; a suitable syntax for add/remove can come later if it presents itself.

jacobkm: Yeah, that's where we're at right now.

were it up to me, I'd just disable add/remove on m2m's with intermediary models and get the rest working and checked in. then work on the rest

fair enough, but i have been using the patch and am dealing with this problem ;)

well i already dealt with it, but trying to help otu

jacobkm: That's the current solution. In fact, the methods are dynamically removed so that autocompletion from some IDEs won't even suggest it.

jacobkm: (As per Russ's request)

Ah, so you're more talking about where to go next.

jacobkm: Yeah, because not having add/remove is kind of annoying, however, there doesn't seem to be a very good solution :(

ericflo: yeah i am not trying to implement a solution now. id be happy if it went in now :)

constructing the intermediary model by hand seems simple enough.

Another option would be to expose the intermediary model through a related manager...

So something like Person.membership.all() / Group.membership.all()

hmmm...

There'd obviously need to be some sort of an "intermediary manager name" attribute so that you don't get clashes on multiple bits.

Well since you already have to put foreign keys, you already get the related manager from that on particular instances.

Ah, right.

So person.membership_set.all()

Nice use of the time machine there, ericflo :)

jacobkm: thanks, heh

jacobkm: I mean, like brosner was saying before, querying is pretty much taken care of, it's the CUD of CRUD that's wonky still.

meh, it's just a little more verbose

jacobkm: I'll massage the patch a little tonight as per some of Russ's suggestions, but if you're just looking for what you said earlier, it's ready.

ericflo: yeah; it worksforme pretty well. I'll give Russ a chance to review that little changes you make, but if he's busy I can check it in soonish too. Bug me next week if you've updated the patch and not gotten any review, OK?

jacobkm: Sounds good.

So the patch is against newforms-admin? Is that right?

Er, what I mean is: do you have it against trunk, too? Or is there a reason not to apply it to trunk?

jacobkm: I had been maintaining patches against pre-qsrf trunk, qsrf, and newforms admin, but it hasn't been updated since PyCon due to other stuff getting in the way.

jacobkm: Tonight I'll bring it up to trunk.

apply against trunk. there is one bit for nfa that removes the m2m. i can apply that once merged over

That's what I'd like to do, yeah.

Cool.

ericflo: i updated after pycon

Thanks brosner :)

actually after some qs-rf stuff you did i think

brosner: With the state of the patch, is there currently no way to get an inline view working of the intermediary stuff in newforms admin?

well it is done explicitly

just make an inline on the intermediary model

brosner: Oh I see, awesome!

brosner: I think that's a pretty decent solution, unless it could somehow be automated for the most basic case?

i wonder if that could be done automatically. maybe not worth it

yeah if we did it implicitly it might cause some headaches to get more customized unless it could be easily overridden which at this point would be nasty

brosner: Ok then let's forget about it, explicit is good

yep. agreed.

that was my comment near the end

Gah gotta go, dad's birthday. I'll get #6095 up to speed tonight.

thanks ericflo