You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@superset.apache.org by "squalou (via GitHub)" <gi...@apache.org> on 2023/03/14 21:02:59 UTC

[GitHub] [superset] squalou opened a new issue, #23375: Datatype not correctly exported (example to xlsx)

squalou opened a new issue, #23375:
URL: https://github.com/apache/superset/issues/23375

   Seen this when exporting to xslx. Some numeric fields ends up as numeric format, some ends as strings. It may depend on column but I did not find a logical pattern.
   
   
   #### How to reproduce the bug
   
   1. Create a dataset based on a table including some numeric decimal fields
   2. Create custom metrics in this dataset (for instance CAST(SUM(my_column) AS DECIMAL 9,2))
   3. Create a table chart based on this dataset, include fields, add the custom metric as aggregate function
   4. Click on Download / Export to Xlsx
   5. Open the xlsx : at least the column obtained from the custom metric will be considered as string
   6. ... some other columns (shown with '#' type in superset UI) sometimes end up as string too
   
   ### Expected results
   
   Ideally I'd like to have 
   - numeric data column end up as numeric 
   - be able to set Metric datatype, and have them as numeric in the end
   
   ### Actual results
   
   some values are string.
   
   
   #### Screenshots
   
   a custom metric
   ![image](https://user-images.githubusercontent.com/4623644/225134676-85b3a594-c5e3-4a9d-97a0-427e8bbf8269.png)
   
   some data in superset
   ![image](https://user-images.githubusercontent.com/4623644/225134879-f352a204-ff32-424c-b045-13195e4af966.png)
   
   result after export
   
   ![image](https://user-images.githubusercontent.com/4623644/225134957-6d80f90a-409c-48d5-898f-4b7b4a178c28.png)
   
   Due to fr locale it's easy to see the numerci fields : they have 'commas' in them. (and string are left aligned)
   
   ### Environment
   
   (please complete the following information):
   
   - browser type and version: Firefox 110
   - superset version: `superset version` : 2.1.0-rc2
   - python version: `python --version` : 3.8.16
   - node.js version: `node -v` : 
   - any feature flags active: none
   
   ### Checklist
   
   Make sure to follow these steps before submitting your issue - thank you!
   
   - [x] I have checked the superset logs for python stacktraces and included it here as text if there are any.
   - [x] I have reproduced the issue with at least the latest released version of superset.
   - [x] I have checked the issue tracker for the same issue and I haven't found one similar.
   
   ### Additional context
   
   I tried to play around with EXCEL_EXPORT config according to padans documentation, but there's no solution here.
   
   I extracted sheet.xml from the exported xlsx to double check that some fields are indeed noted as strings ("s"), and some other contain numeric value.
   
   
   
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] squalou commented on issue #23375: Datatype not correctly exported to xlsx - with patch suggestion

Posted by "squalou (via GitHub)" <gi...@apache.org>.
squalou commented on issue #23375:
URL: https://github.com/apache/superset/issues/23375#issuecomment-1473268704

   Here's the illustrated fix
   
   https://github.com/squalou/superset/commit/f2768e6f70778388f9f18a76ac8ab9b6cc14869d
    
    The thing that matters the most is the result *inside* the xlsx file, there's a sheet1.xml that must contains numeric values and not string whenever the datasource says it's numeric, which is not the case atoday.
    
    Tricky part is that when decimal separator matches (i.e. when testing win ud_en locale for instance), a number written as a *string* in the xlsx file will be *renderd* as a usable number in Excel, making the issue hard to notice.
    
   
    


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] squalou commented on issue #23375: Datatype not correctly exported to xlsx - with patch suggestion

Posted by "squalou (via GitHub)" <gi...@apache.org>.
squalou commented on issue #23375:
URL: https://github.com/apache/superset/issues/23375#issuecomment-1481759761

   first attempt 
   https://github.com/apache/superset/pull/23476


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


Re: [I] Datatype not correctly exported to xlsx - with patch suggestion [superset]

Posted by "nandwalritik (via GitHub)" <gi...@apache.org>.
nandwalritik commented on issue #23375:
URL: https://github.com/apache/superset/issues/23375#issuecomment-1954123531

   Hi @squalou were you able to fix this issue? I saw your PR but its still open.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


Re: [I] Datatype not correctly exported to xlsx - with patch suggestion [superset]

Posted by "squalou (via GitHub)" <gi...@apache.org>.
squalou commented on issue #23375:
URL: https://github.com/apache/superset/issues/23375#issuecomment-1960962214

   well, I'm running this patch on my superset indtance for months. (testing on 4.0.0-rc1 at the moment)
   
   I can rebase - re submit the PR but no idea how to have it accepted.
   
   Maybe - considering the mess I did at the beginning - I should reopen a new one ? I don't know what are the best practices


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


Re: [I] Datatype not correctly exported to xlsx - with patch suggestion [superset]

Posted by "rusackas (via GitHub)" <gi...@apache.org>.
rusackas closed issue #23375: Datatype not correctly exported  to xlsx - with patch suggestion
URL: https://github.com/apache/superset/issues/23375


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] EugeneTorap commented on issue #23375: Datatype not correctly exported to xlsx - with patch suggestion

Posted by "EugeneTorap (via GitHub)" <gi...@apache.org>.
EugeneTorap commented on issue #23375:
URL: https://github.com/apache/superset/issues/23375#issuecomment-1481710673

   Hi @squalou! Thanks for this issue! Can you prepare a PR with these changes?
   Cc: @villebro @dpgaspar


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] squalou commented on issue #23375: Datatype not correctly exported to xlsx - with patch suggestion

Posted by "squalou (via GitHub)" <gi...@apache.org>.
squalou commented on issue #23375:
URL: https://github.com/apache/superset/issues/23375#issuecomment-1481742601

   Hi, I'm not sure I know how to, but I'll have a look and try :-)


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] squalou commented on issue #23375: Datatype not correctly exported to xlsx - with patch suggestion

Posted by "squalou (via GitHub)" <gi...@apache.org>.
squalou commented on issue #23375:
URL: https://github.com/apache/superset/issues/23375#issuecomment-1471906217

   patch idea : 
   forward the guessed datatypes to excel export code.
   
   ```py
   diff query_actions.py query_actions-patch.py                                                     
   110c110
   <         payload["data"] = query_context.get_data(df)
   ---
   >         payload["data"] = query_context.get_data(df, payload["coltypes"])
   ```
   
   ```py
   diff query_context.py query_context-patch.py                                                     
   90a91
   >         coltypes:[] = None
   92c93
   <         return self._processor.get_data(df)
   ---
   >         return self._processor.get_data(df,coltypes)
   ```
   
   ```py
   diff query_context_processor.py query_context_processor-patch.py                                 
   18d17
   < 
   62a62
   >     GenericDataType
   450c450
   <     def get_data(self, df: pd.DataFrame) -> Union[str, List[Dict[str, Any]]]:
   ---
   >     def get_data(self, df: pd.DataFrame, coltypes:[]) -> Union[str, List[Dict[str, Any]]]:
   464c464,474
   <                 result = excel.df_to_excel(df, **config["EXCEL_EXPORT"])
   ---
   >                 ndf = df.copy()
   >                 columns = list(df)
   >                 i = 0
   >                 for c in columns:
   >                     coltype = None
   >                     if i<len(coltypes):
   >                         coltype = coltypes[i]
   >                         if coltype == GenericDataType.NUMERIC:
   >                             ndf[c] = pd.to_numeric(df[c], errors='ignore')
   >                     i+=1
   >                 result = excel.df_to_excel(ndf, **config["EXCEL_EXPORT"])
   
   ```
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] squalou commented on issue #23375: Datatype not correctly exported to xlsx - with patch suggestion

Posted by "squalou (via GitHub)" <gi...@apache.org>.
squalou commented on issue #23375:
URL: https://github.com/apache/superset/issues/23375#issuecomment-1471707508

   I think I get the reason ... there's a logic to guess data types from datasource at some point in query_actions.py, calling core.py  `extract_dataframe_dtypes(df, datasource)`
   which seems fine, 
   but is never passed to the code part that actually *does* the xls export.
   
   will try harder to fix that on my side


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] EugeneTorap commented on issue #23375: Datatype not correctly exported to xlsx - with patch suggestion

Posted by "EugeneTorap (via GitHub)" <gi...@apache.org>.
EugeneTorap commented on issue #23375:
URL: https://github.com/apache/superset/issues/23375#issuecomment-1481756898

   @squalou https://preset.io/blog/tutorial-contributing-code-to-apache-superset/


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] squalou commented on issue #23375: Datatype not correctly exported (example to xlsx)

Posted by "squalou (via GitHub)" <gi...@apache.org>.
squalou commented on issue #23375:
URL: https://github.com/apache/superset/issues/23375#issuecomment-1469756997

   I *would* suggest a quick fix, but not sure I can manage a pull request
   
   here's the idea in `superset/common/query_context_processor.py`
   
   before
   ```py
   result = excel.df_to_excel(df, **config["EXCEL_EXPORT"])
   ```
   after
   
   ```py
   ndf = df.copy()                
   columns = list(df)
   for c in columns:
       logging.info(f"column {c} dtype is {df[c].dtypes}")
       if is_object_dtype(df[c]) and not is_string_dtype(df[c]):
           ndf[c] = pd.to_numeric(df[c], errors='ignore')
           logging.info(f"column {c} converted to numeric")
   
   result = excel.df_to_excel(ndf, **config["EXCEL_EXPORT"])
   
   ```
   
   This way, data not explicetely typed as string or dates or numeric ... get a chance to be silentely converted to a numeric.
   
   I have good results with this, maybe there's a more elegant way to proceed (especially no idea if doing a copy is useful as I don't know the big picture)
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org