Fix field distribution arg in GaussianCopula #743
Conversation
sdv/tabular/copulas.py
Outdated
| self._field_distributions[column] = self._default_distribution | ||
| if column not in self._field_distributions: | ||
| # Check if the column is a derived column. | ||
| column_name = column.replace('.value', '').replace('.is_null', '') |
There was a problem hiding this comment.
The is_null columns are just boolean columns that say whether or not their row should be null. I would apply a fixed distribution to them.
I think it only makes sense to apply the specified distribution to the one ending in .value
Codecov Report
@@ Coverage Diff @@
## master #743 +/- ##
==========================================
+ Coverage 66.76% 67.15% +0.39%
==========================================
Files 36 38 +2
Lines 2738 3075 +337
==========================================
+ Hits 1828 2065 +237
- Misses 910 1010 +100
Continue to review full report at Codecov.
|
| if column not in self._field_distributions: | ||
| # Check if the column is a derived column. | ||
| column_name = column.replace('.value', '') | ||
| self._field_distributions[column] = self._field_distributions.get( |
There was a problem hiding this comment.
One thing to note is that the _field_distributions dictionary will have some field names that match the RDT output, and some that match the input. For example, if null columns are created and there is a column 'a', then the dictionary will have
{
'a': dist,
'a.is_null': default_dist
}
Idk if it makes sense to have them all match the HyperTransformer output names or not (ie. keep the .value extension)
There was a problem hiding this comment.
I created #744 to track this question, not completely sure what we should do right now.
Resolves #746