Add documentation for fit_column_model and update default #549
Conversation
katxiao
changed the title
katxiao
force-pushed
the
sdv-issue-517-fit-columns-model-doc
branch
from
4bfa5d0 to
234b826
Compare
| @@ -226,6 +227,14 @@ def _validate_constraint_columns(self, table_data): | |||
| """ | |||
| missing_columns = [col for col in self.constraint_columns if col not in table_data.columns] | |||
| if missing_columns: | |||
| if not self._columns_model: | |||
| warning_message = ( | |||
There was a problem hiding this comment.
Could we add a small unit test for this?
There was a problem hiding this comment.
Yes, I just added it!
There was a problem hiding this comment.
Now that I look at this, I wonder if it is confusing to a user. @csala The case where this happens is usually because of conditioning right? Would they be confused to see a warning that says "constraints are missing from table data". Should we say that it's because of conditioning to provide more context?
There was a problem hiding this comment.
Hm that's a good point. I didn't completely understand the use case when I wrote this message.
I can update the comment to say, "When fit_columns_model is False and we are conditioning on a subset of the constraint columns, reject sampling can become slow."
What do you think?
There was a problem hiding this comment.
I think that makes sense
| @@ -226,6 +227,14 @@ def _validate_constraint_columns(self, table_data): | |||
| """ | |||
| missing_columns = [col for col in self.constraint_columns if col not in table_data.columns] | |||
| if missing_columns: | |||
| if not self._columns_model: | |||
| warning_message = ( | |||
There was a problem hiding this comment.
Now that I look at this, I wonder if it is confusing to a user. @csala The case where this happens is usually because of conditioning right? Would they be confused to see a warning that says "constraints are missing from table data". Should we say that it's because of conditioning to provide more context?
katxiao
force-pushed
the
sdv-issue-517-fit-columns-model-doc
branch
from
c16bcd2 to
2d03720
Compare
katxiao
force-pushed
the
sdv-issue-517-fit-columns-model-doc
branch
from
2d03720 to
39af260
Compare
Add documentation for fit_column_model and change the default value to
False. Also add a warning for when reject sampling may take a long timeResolves #517, #522, and #550