Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[DeckGL] Raise error with null values #5302

Merged
merged 6 commits into from
Jun 30, 2018
Merged

[DeckGL] Raise error with null values #5302

merged 6 commits into from
Jun 30, 2018

Conversation

hughhhh
Copy link
Member

@hughhhh hughhhh commented Jun 27, 2018

Really vague message with null values. Want to be more transparent about the errors in the underlying code for showing data in deckgl

@mistercrunch

superset/viz.py Outdated
@@ -2107,6 +2108,10 @@ def tupleify(s):
df[key] = list(zip(latlong.apply(lambda x: x[0]),
latlong.apply(lambda x: x[1])))
del df[spatial.get('geohashCol')]

if df[key] is None:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(optional)df.get(key) is None: would protect against KeyError (not sure if they're possible in this context)

superset/viz.py Outdated
@@ -2107,6 +2108,10 @@ def tupleify(s):
df[key] = list(zip(latlong.apply(lambda x: x[0]),
latlong.apply(lambda x: x[1])))
del df[spatial.get('geohashCol')]

if df[key] is None:
raise NullValueException('Some rows in this query contain NULL values!')
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Msg could be a bit more precise and use i18n wrapper: _(Encountered invalid NULL spatial entry, please consider filtering those out)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@vylc wording ok?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a python lib for i18n? i thought is was only available for JS, i was searching the code for some example and couldn't find any.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

in this module it's available from the line from flask_babel import lazy_gettext as _

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the setup is kind of sweet where we use the same framework for both the backend and frontend.

@mistercrunch
Copy link
Member

LGTM

@codecov-io
Copy link

Codecov Report

Merging #5302 into master will increase coverage by 0.01%.
The diff coverage is 80%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #5302      +/-   ##
==========================================
+ Coverage   61.31%   61.32%   +0.01%     
==========================================
  Files         369      369              
  Lines       23468    23488      +20     
  Branches     2717     2717              
==========================================
+ Hits        14390    14405      +15     
- Misses       9066     9071       +5     
  Partials       12       12
Impacted Files Coverage Δ
superset/exceptions.py 100% <100%> (ø) ⬆️
superset/viz.py 81.35% <66.66%> (-0.03%) ⬇️
superset/sql_lab.py 71.6% <0%> (-3.4%) ⬇️
superset/dataframe.py 94.49% <0%> (-3.21%) ⬇️
superset/db_engine_specs.py 54.28% <0%> (+1.11%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 04fc1d1...1717e4a. Read the comment docs.

@hughhhh hughhhh merged commit 089037f into apache:master Jun 30, 2018
mistercrunch pushed a commit to lyft/incubator-superset that referenced this pull request Jul 2, 2018
* raise errors with null values

* linting

* linting some more

* use get

* change ordering

* linting

(cherry picked from commit 089037f)
mistercrunch pushed a commit that referenced this pull request Jul 3, 2018
* raise errors with null values

* linting

* linting some more

* use get

* change ordering

* linting

(cherry picked from commit 089037f)
hughhhh added a commit to lyft/incubator-superset that referenced this pull request Jul 5, 2018
* raise errors with null values

* linting

* linting some more

* use get

* change ordering

* linting

(cherry picked from commit 089037f)
hughhhh added a commit to lyft/incubator-superset that referenced this pull request Jul 11, 2018
* raise errors with null values

* linting

* linting some more

* use get

* change ordering

* linting

(cherry picked from commit 089037f)
hughhhh added a commit to lyft/incubator-superset that referenced this pull request Jul 11, 2018
* raise errors with null values

* linting

* linting some more

* use get

* change ordering

* linting

(cherry picked from commit 089037f)
mistercrunch pushed a commit to lyft/incubator-superset that referenced this pull request Jul 11, 2018
* raise errors with null values

* linting

* linting some more

* use get

* change ordering

* linting

(cherry picked from commit 089037f)
mistercrunch pushed a commit to lyft/incubator-superset that referenced this pull request Jul 11, 2018
* raise errors with null values

* linting

* linting some more

* use get

* change ordering

* linting

(cherry picked from commit 089037f)
mistercrunch pushed a commit to lyft/incubator-superset that referenced this pull request Jul 11, 2018
* raise errors with null values

* linting

* linting some more

* use get

* change ordering

* linting

(cherry picked from commit 089037f)
hughhhh added a commit to lyft/incubator-superset that referenced this pull request Jul 11, 2018
* raise errors with null values

* linting

* linting some more

* use get

* change ordering

* linting

(cherry picked from commit 089037f)
timifasubaa pushed a commit to airbnb/superset-fork that referenced this pull request Jul 25, 2018
* raise errors with null values

* linting

* linting some more

* use get

* change ordering

* linting
mistercrunch pushed a commit to lyft/incubator-superset that referenced this pull request Jul 26, 2018
* raise errors with null values

* linting

* linting some more

* use get

* change ordering

* linting

(cherry picked from commit 089037f)
mistercrunch pushed a commit to lyft/incubator-superset that referenced this pull request Jul 26, 2018
* raise errors with null values

* linting

* linting some more

* use get

* change ordering

* linting

(cherry picked from commit 089037f)
mistercrunch pushed a commit to lyft/incubator-superset that referenced this pull request Jul 26, 2018
* raise errors with null values

* linting

* linting some more

* use get

* change ordering

* linting

(cherry picked from commit 089037f)
mistercrunch pushed a commit to lyft/incubator-superset that referenced this pull request Jul 27, 2018
* raise errors with null values

* linting

* linting some more

* use get

* change ordering

* linting

(cherry picked from commit 089037f)
mistercrunch pushed a commit to lyft/incubator-superset that referenced this pull request Jul 31, 2018
* raise errors with null values

* linting

* linting some more

* use get

* change ordering

* linting

(cherry picked from commit 089037f)
mistercrunch pushed a commit to lyft/incubator-superset that referenced this pull request Aug 3, 2018
* raise errors with null values

* linting

* linting some more

* use get

* change ordering

* linting

(cherry picked from commit 089037f)
mistercrunch pushed a commit to lyft/incubator-superset that referenced this pull request Aug 3, 2018
* raise errors with null values

* linting

* linting some more

* use get

* change ordering

* linting

(cherry picked from commit 089037f)
mistercrunch pushed a commit to lyft/incubator-superset that referenced this pull request Aug 4, 2018
* raise errors with null values

* linting

* linting some more

* use get

* change ordering

* linting

(cherry picked from commit 089037f)
mistercrunch pushed a commit to lyft/incubator-superset that referenced this pull request Aug 4, 2018
* raise errors with null values

* linting

* linting some more

* use get

* change ordering

* linting

(cherry picked from commit 089037f)
wenchma pushed a commit to wenchma/incubator-superset that referenced this pull request Nov 16, 2018
* raise errors with null values

* linting

* linting some more

* use get

* change ordering

* linting
@mistercrunch mistercrunch added 🍒 0.26.0 🍒 0.26.1 🍒 0.26.2 🍒 0.26.3 🍒 0.27.0 🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels 🚢 0.28.0 labels Feb 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels 🍒 0.26.0 🍒 0.26.1 🍒 0.26.2 🍒 0.26.3 🍒 0.27.0 🚢 0.28.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants