[GitHub] mistercrunch commented on a change in pull request #4905: [wip][missing values] Removing replacing missing values

2018-04-30 Thread GitBox
mistercrunch commented on a change in pull request #4905: [wip][missing values] 
Removing replacing missing values
URL: 
https://github.com/apache/incubator-superset/pull/4905#discussion_r185045726
 
 

 ##
 File path: superset/viz.py
 ##
 @@ -961,7 +936,6 @@ def as_floats(field):
 return d
 
 def get_data(self, df):
-df = df.fillna(0)
 
 Review comment:
   Are we sure that this particular visualization handles null nicely?


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] mistercrunch commented on a change in pull request #4905: [wip][missing values] Removing replacing missing values

2018-04-30 Thread GitBox
mistercrunch commented on a change in pull request #4905: [wip][missing values] 
Removing replacing missing values
URL: 
https://github.com/apache/incubator-superset/pull/4905#discussion_r185045338
 
 

 ##
 File path: superset/viz.py
 ##
 @@ -123,24 +122,6 @@ def run_extra_queries(self):
 """
 pass
 
-def get_fillna_for_col(self, col):
-"""Returns the value for use as filler for a specific Column.type"""
-if col:
-if col.is_string:
-return ' NULL'
 
 Review comment:
   I think @xrmx added some of this logic. `null`-labeled series creates issues 
on some visualizations. I believe the leading space has to do with sorting NULL 
first. We may want to reuse some of the logic I added recently around filtering 
that replaces `None` by  `` and empty stings by ``. This 
should only be applied to dimensions / strings.  Note that it applies only to 
string columns, thus not "zerofying"  NULL values.
   
   I'm pretty sure that removing this will break many charts.


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services