Refs -- #18597 feat(BaseModelFormSet): Cleanup queryset prep in init and instance fetching

[agalazis]

cleanup related to enabling custom prefetched inlines see https://code.djangoproject.com/ticket/18597

[agalazis]

@WhyNotHugo want to have a look?

[WhyNotHugo]

Makes sense to me. I assume the intent is to follow up with a PR that changes _get_instance_by_queryset_index to use a pre-fetched queryset.

[agalazis]

yes but also allow overriding it in general without rewriting a lot of methods(and also not rewriting the whole constructor just to overwrite how queryset is prepared)

[WhyNotHugo]

If the intent is for it to be overridden, then I don't think the name should start with _. This is usually an indicator that it's a private API, not intended to be used by external consumers, nor promising any stability.

[agalazis]

I was more interested denote that they are protected and should only be used by subclasses. I was having in mind _ for protected vs __ for private but you know better the coding style of this project and whether it is ok to be entirely public. I will change it this afternoon, thanks.

[agalazis]

fixed @WhyNotHugo feel free to have a look

[WhyNotHugo]

Everything looks good to me, but I'm not a maintainer.

Maybe mention it here to get more eyes on it?

[auvipy]

the thing that is missing is adding unit test for the changes

[WhyNotHugo]

This is already covered by tests, the refactor doesn't really add new functionality to test, right?

[felixxm]

This is already covered by tests, the refactor doesn't really add new functionality to test, right?

This PR adds new hooks to be overridden by subclasses. We should test if they can be used in the described scenario with prefetch_related(). Moreover new hooks without docs, release notes, and tests are not discoverable, so we would only add them for few folks, which is not our intention.

[WhyNotHugo]

Agreed. Docs are also important here.

[felixxm]

@agalazis Do you have time to keep working on this?

[agalazis]

Yes I ll be back on it within next 2 weeks

[felixxm]

Closing due to inactivity.