Skip to content

Grid Accept Dataframe #675

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

Merged
merged 3 commits into from
Jan 27, 2017
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 17 additions & 2 deletions plotly/grid_objs/grid_objs.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,9 @@

from requests.compat import json as _json

from plotly import exceptions, utils
from plotly import exceptions, optional_imports, utils

pd = optional_imports.get_module('pandas')
Copy link
Contributor

Choose a reason for hiding this comment

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

👌


__all__ = None

Expand Down Expand Up @@ -148,7 +150,20 @@ def __init__(self, columns_or_json, fid=None):
```
"""
# TODO: verify that columns are actually columns
if isinstance(columns_or_json, dict):
if pd and isinstance(columns_or_json, pd.DataFrame):
duplicate_name = utils.get_first_duplicate(columns_or_json.columns)
if duplicate_name:
err = exceptions.NON_UNIQUE_COLUMN_MESSAGE.format(duplicate_name)
raise exceptions.InputError(err)

# create columns from dataframe
all_columns = []
for name in columns_or_json.columns:
all_columns.append(Column(columns_or_json[name].tolist(), name))
self._columns = all_columns
self.id = ''

elif isinstance(columns_or_json, dict):
# check that fid is entered
if fid is None:
raise exceptions.PlotlyError(
Expand Down
Empty file.
32 changes: 32 additions & 0 deletions plotly/tests/test_optional/test_grid/test_grid.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
"""
test_grid:
==========

A module intended for use with Nose.

"""
from __future__ import absolute_import

from unittest import TestCase

from plotly.exceptions import InputError
from plotly.grid_objs import Grid

import pandas as pd


class Test_Dataframe_to_Grid(TestCase):
Copy link
Contributor

Choose a reason for hiding this comment

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

pep:8ball: haha class names should be like TestDataframeToGrid or something. This is like 🐍 + 🐫 or something.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Two things:

  1. What does 🐍 mean?
  2. Why would DRY be relevant here? What am I repeating?

I'm changing it to your suggested class name, just curious

Copy link
Contributor

Choose a reason for hiding this comment

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

Sorry, I can see why that's confusing ;__;.

I was talking about camelCase vs snake_case (hence the 🐍 and 🐫).


# Test duplicate columns
def test_duplicate_columns(self):
df = pd.DataFrame([[1, 'a'],
[2, 'b']], columns=['col_1', 'col_1'])
Copy link
Contributor

Choose a reason for hiding this comment

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

🐄 this is a weird way to break this line, I'd suggest:

df = pd.DataFrame([[1, 'a'], [2, 'b']],
                  columns=['col_1', 'col_1'])


NON_UNIQUE_COLUMN_MESSAGE = (
Copy link
Contributor

Choose a reason for hiding this comment

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

🐄 no real need for this to be a CONSTANT, imo. no biggie though.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Do you mean rename it to something like pattern or message rather than ALLCAPSCONSTANT?

Copy link
Contributor

Choose a reason for hiding this comment

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

expected_message or something is what I meant.

"Yikes, plotly grids currently "
"can't have duplicate column names. Rename "
"the column \"{}\" and try again.".format('col_1')
)

with self.assertRaisesRegexp(InputError, NON_UNIQUE_COLUMN_MESSAGE):
Grid(df)