You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@superset.apache.org by GitBox <gi...@apache.org> on 2020/03/16 21:58:17 UTC
[GitHub] [incubator-superset] mistercrunch opened a new pull request #9314:
fix: big number to handle NULL as it did in the past
mistercrunch opened a new pull request #9314: fix: big number to handle NULL as it did in the past
URL: https://github.com/apache/incubator-superset/pull/9314
context: https://github.com/apache/incubator-superset/pull/9313
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] etr2460 commented on issue #9314: fix: big
number to handle NULL as it did in the past
Posted by GitBox <gi...@apache.org>.
etr2460 commented on issue #9314: fix: big number to handle NULL as it did in the past
URL: https://github.com/apache/incubator-superset/pull/9314#issuecomment-600183820
@mistercrunch is this good to merge now?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] mistercrunch commented on issue #9314: fix:
big number to handle NULL as it did in the past
Posted by GitBox <gi...@apache.org>.
mistercrunch commented on issue #9314: fix: big number to handle NULL as it did in the past
URL: https://github.com/apache/incubator-superset/pull/9314#issuecomment-599777539
I can write a unit test covering `get_data` later tonight
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] willbarrett commented on issue #9314: fix: big
number to handle NULL as it did in the past
Posted by GitBox <gi...@apache.org>.
willbarrett commented on issue #9314: fix: big number to handle NULL as it did in the past
URL: https://github.com/apache/incubator-superset/pull/9314#issuecomment-599794445
@etr2460 unfortunately I was unable to come up with a comprehensive unit test by the end of my day. This will have to wait for @mistercrunch to free up again this evening. Apologies.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] willbarrett commented on issue #9314: fix: big
number to handle NULL as it did in the past
Posted by GitBox <gi...@apache.org>.
willbarrett commented on issue #9314: fix: big number to handle NULL as it did in the past
URL: https://github.com/apache/incubator-superset/pull/9314#issuecomment-599783660
I'm working on a unit test for this PR now.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] codecov-io commented on issue #9314: fix: big
number to handle NULL as it did in the past
Posted by GitBox <gi...@apache.org>.
codecov-io commented on issue #9314: fix: big number to handle NULL as it did in the past
URL: https://github.com/apache/incubator-superset/pull/9314#issuecomment-599887303
# [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/9314?src=pr&el=h1) Report
> Merging [#9314](https://codecov.io/gh/apache/incubator-superset/pull/9314?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-superset/commit/f1370c5e324d7817b0fda92079178440c0832081&el=desc) will **not change** coverage by `%`.
> The diff coverage is `n/a`.
[](https://codecov.io/gh/apache/incubator-superset/pull/9314?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #9314 +/- ##
=======================================
Coverage 59.08% 59.08%
=======================================
Files 374 374
Lines 12205 12205
Branches 2989 2989
=======================================
Hits 7211 7211
Misses 4815 4815
Partials 179 179
```
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-superset/pull/9314?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/9314?src=pr&el=footer). Last update [f1370c5...06eb8ea](https://codecov.io/gh/apache/incubator-superset/pull/9314?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] mistercrunch merged pull request #9314: fix:
big number to handle NULL as it did in the past
Posted by GitBox <gi...@apache.org>.
mistercrunch merged pull request #9314: fix: big number to handle NULL as it did in the past
URL: https://github.com/apache/incubator-superset/pull/9314
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] mistercrunch commented on issue #9314: fix:
big number to handle NULL as it did in the past
Posted by GitBox <gi...@apache.org>.
mistercrunch commented on issue #9314: fix: big number to handle NULL as it did in the past
URL: https://github.com/apache/incubator-superset/pull/9314#issuecomment-599885216
Definitely was worth taking the time to write the unit test. Pandas is doing some funky things here in `pivot_table`. While clearly `np.isnan(np.sum(np.nan)) == True` `df.pivot_table({...}, aggfunc=np.sum)` of a column that has only `None` returns 0.
Anyhow. This here works, I picked `np.min` as no aggregates apply (already at the right grain) and with `np.min` a `None` leads to `nan`, which works well in the end (same as before the original PR). I tested this same unit test on before this PR and it fails (as expected, with the `0` value in place of the `None/nan/NULL`), and tested on before the a commit prior to the PR that created the problem `753aeb482991f0dd3f36373f6d248ce277c67949` and this unit test passes. This gives pretty high confidence that `None` / `NULL` will be treated the same as before.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org