Add check for all missing arguments to schema.

[haberdashPI]

A common mistake in using Legolas is to call MySchemaV1() instead of MySchemaV1SchemaVersion(). Since calling MySchemaV1() is rarely intentional, this PR flags a warning when that method is called. The warning can be silenced by calling MySchemaV1(Legolas.AllMissing()) to signal that, yes, you really do want a record with all missing fields.

[ericphanson]

IMO there shouldn't be any warning if I do MySchema(row_with_all_missing) or MySchema(; col1=missing, col2=missing). I could see the rationale for a literal invocation of MySchema() to warn though.

[iamed2]

As an addendum to @ericphanson's statement, I think MySchemaV1((;)) should not warn.

But I wonder if this is best solved in a method signature. Are there cases where it makes sense to have an identical method signature that differs only between ::SchemaVersion and ::AbstractRecord in one parameter, where the result would be different?

[haberdashPI]

Not sure I follow @iamed2:

But I wonder if this is best solved in a method signature.

Are you saying that downstream consumers should be able to handle both MySchemaV1 and MySchemaV1SchemaVersion?

I think the issue is that you can get an error when using MySchemaV1() (because one of the fields is not allowed to be missing). This is often a confusing error message, and part of the role of making the method warn/error would be to make the message users get more legible.

[ericphanson]

I think @iamed2 is suggesting method authors should take care to avoid this problem by type annotations in their methods signatures. I agree that would be good too, but I think suitably restricted this could be a reasonable fallback. FWIW a culprit of bad method signatures in this department is a particular closed source database client.

[haberdashPI]

Okay, thanks for clarifying. Sounds like I need to adjust this PR by:

• remove the check for all missing keyword arguments
• replace with a check for an empty list of keyword arguments

[haberdashPI]

As a reminder to myself: still todo is to make a method for MySchemaV1((;)) so that it doesn't error (probably by just passing missing instead of Legoals.DefaultArgument).

[ericphanson]

Hm, is it feasible to add a second constructor with no arguments and make that one warn? That might be simpler, but I suppose it’s only valid if every field is nullable

[haberdashPI]

Hm, is it feasible to add a second constructor with no arguments and make that one warn? That might be simpler, but I suppose it’s only valid if every field is nullable

It's funny because I wrote a reply saying that yes, we could simplify this the way you suggest, and then I remembered why we can't when I tried to implement it.

The method $R(; $(field_kwargs...)) overlaps with $R(), so by defining an empty constructor, we're actually defining the same method twice. Probably good to write a comment about that rationale.