Outstanding issues in composer

[fsaad]

According @axch

• Um, include the casefold in the presence check at composer.py:178, if that's what you meant.
• Line 207: What are locls, fcols, pcols, and fpreds? Can we make the names more descriptive?
  • Might not be a bad idea to define a container class for the "hodgepodge", named something like "Configuration" or "Schema".
  • Actually, a namedtuple will probably do
• What's with the comment on line 215? Is that an example value? If so, call it that and fix it (underscore)
• Line 230: Why is this a bql query instead of a call to the metamodel API?
  • If it remains a BQL query, should quote the elements properly
• Line 305: Quote the cc_name
• Line 314: Why is this a BQL query intead of a call to the metamodel API?
  • Is there any robust way to pass the set of modelnos through, to maintain the intended 1-1 mapping between composer model numbers and underlying crosscat model numbers?
• Line 514: Is this actually correct? Looks like a type error: checking whether a tuple is in "ignore", even though that accumulates just the columns. Would manifest as failing to ignore things it should ignore.
  • If you are not sure, this would be a good thing to poke with a test.
  • We also know how to build the ignore table better, per Taylor's review of predictive_probability.

[fsaad]

Um, include the casefold in the presence check at composer.py:178, if that's what you meant. Will fix.

Line 207: What are locls, fcols, pcols, and fpreds? Can we make the names more descriptive? The docstring for parse contains all the definitions.

What's with the comment on line 215? Is that an example value? If so, call it that and fix it (underscore) Should remove -- artifact of old code that hardcored the schema.

Line 230: Why is this a bql query instead of a call to the metamodel API? Must be a BQL query since calling CREATE GENERATOR does an array of important stuff to make the sure the generator is successfully initialized before calling the create_generator function of the metamodel. Invoking this method directly results a half-baked generator.

Line 514: Is this actually correct? No, it is a bug.

_We also know how to build the ignore table better, per Taylor's review of predictiveprobability. I implemented the 'better' implementation, if better means O(m+n) not O(mn), in the crosscat code before merging that branch. Can migrate same algorithm here if required.

[fsaad]

If it remains a BQL query, should quote the elements properly Should make sure we use the table name, not satellites!

[gregory-marton]

@fsaad, status update?

[fsaad]

No updates, I took a hiatus from composer since writing those comments. These issues are likely to be revisited in the near future