More powerful column selection in MastMissions#3492
Conversation
bsipocz
left a comment
bsipocz
left a comment
There was a problem hiding this comment.
Overall this looks good, but I wonder if the try/except exceptions can be better specified rather than blanket everything.
|
|
||
| if isinstance(select_cols, str): | ||
| select_cols = select_cols.split(',') | ||
| elif not isinstance(select_cols, list): |
There was a problem hiding this comment.
nitpicking, but could it be other reasonable iterable, too, e.g. why don't we accept tuples or even a generator?
| data_table.add_column( | ||
| MaskedColumn(coerced, name=col_name, mask=ignore_mask) | ||
| ) | ||
| except Exception: |
There was a problem hiding this comment.
should this be a blanket all Exceptions or could we narrow it a bit?
|
|
||
| try: | ||
| out[i] = col_type(val) | ||
| except Exception: |
There was a problem hiding this comment.
again, this should be done with a very specific exception only
snbianco
force-pushed
the
select-all-cols
branch
from
bd820cd to
dafded1
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #3492 +/- ##
==========================================
+ Coverage 71.83% 71.95% +0.12%
==========================================
Files 235 235
Lines 20273 20328 +55
==========================================
+ Hits 14563 14627 +64
+ Misses 5710 5701 -9 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
Made those changes! |
2 participants
Make the
select_colsparameter inMastMissionsqueries more powerful:*orallI have also made the
_json_to_tablefunction more robust so that columns that have values of an unexpected type don't raise an error. Instead, a warning is logged and those particular values are masked.