Add support for iterators as m2m kwargs

[ghost]

As detailed in #129, you cannot pass iterators for m2m kwargs when creating objects. Iterators may be useful when you've created a set of objects, and want to assign them one-by-one to the M2M fields of another list of objects.

This changes the behavior such that the iterator is iterated for each object being created, leaving the M2M's with one object each:

class Foo(Model):
    pass

class Bar(Model):
    foos = models.ManyToManyField(Foo)

# Foos in this case may either be 'made' or 'prepared'
foos = baker.make(Foo, _quantity=3)
bars = baker.make(Bar, _quantity=3, foos=iter(foos))

bars[0].foos.first() == foos[0]
bars[1].foos.first() == foos[1]
bars[2].foos.first() == foos[2]

[ghost]

I'm still a bit unfamiliar with the internals of model-bakery, but when I use self.m2m_dict here instead of self.model_attrs I keep hitting StopIteration for my iterator tests

Here it seems that passing the single-object list with self.model_attrs does correctly add the object to the M2M if the model instances in the iterator have been saved

If the model instances in the iterator have not been saved (i.e. test_create_many_to_many_with_unsaved_iter()) it seems to hit _handle_one_to_many(), which is why I copied the if not value.pk: value.save() logic over to there

[timjklein36]

Should this test also assert that the students are persisted once added to classrooms?

[ghost]

When the students are added to the M2M they must be persistent instances, otherwise you hit this exception from the ORM:

I added students[...].pk is not None checks in 294ae92 to clarify that the test expects the students to persist, however if the students were to not persist the test would actually hit the exception shown in my screenshot before hitting the primary key assertions

[berinhard]

Thanks for your PR @lassandroan. I think the code looks code, even though it's confusing. But this is due more by the complexity of fields management by bakery than your work here. Thanks for the time you spent trying to understand this =)

[berinhard]

Once another maintainer approves this PR, we'll merge it and release a new version of the lib.

[berinhard]

Actually, I don't see CI builds results for this PR. Maybe we have something wrong with the workflows files @amureki ?

[timjklein36]

Looks great! Thanks for your contribution, @lassandroan!

[ghost]

Actually, I don't see CI builds results for this PR. Maybe we have something wrong with the workflows files @amureki ?

I don't see it now for some reason, but when I first opened up the PR it showed me some warning saying workflows don't run automatically for first-time contributors

[berinhard]

@lassandroan that seems to be the case, but I really though I triggered the build. Anyway, I know this is dumb, but can you add a new commit to this PR? Just to see if, once this PR has reviews, it triggers CI?

[ghost]

@berinhard added a48fde3 to try triggering it, and now I do see the workflow message again:

[berinhard]

Thanks @lassandroan! Now CI is running

[ghost]

@berinhard looks like one of the commands failed: ERROR: pydocstyle: commands failed, but only for the Python 3.8 check.

Thought I broke the tests but it also seems to be failing in other new pull requests like the Dependabot ones: #239

[timjklein36]

Yeah it looks like the pydocstyle tox environment is actually running the following as its command (which would be the case for anything based on main as of now):

pydocstyle run-test: commands[0] | pytest

Might need to add a [textenv:pydocstyle] section to the tox.ini file. And this does not seem to be an issue with this particular PR.

[berinhard]

You're totally right @timjklein36! I'll try to investigate this tomorrow/friday

[berinhard]

Hi @lassandroan! The PR #241 fixed the error CI is raising for the PRs and #242 makes it easier for us to identify if the PR is blocked due to broken tests or minor linter issues.

Please, can you rebase your branch to the most up to date version of the main one and push force an update this PR? I've already tried to rebuild it manually after the merge of the PRs mentioned, but it doesn't seems to be working.

[ghost]

@berinhard just force pushed, looks like the workflows needs approval still before they can start but I think we should be good this time! 🙂

[berinhard]

merged! thanks for your contribution @lassandroan