Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
13 changes: 3 additions & 10 deletions tableaudocumentapi/datasource.py
Original file line number Diff line number Diff line change
Expand Up @@ -262,14 +262,14 @@ def _get_column_objects(self):
return [_column_object_from_column_xml(self._datasourceTree, xml)
for xml in self._datasourceTree.findall('.//column')]

def add_field(self, name, datatype, role, type, caption):
def add_field(self, name, datatype, role, field_type, caption):
""" Adds a base field object with the given values.

Args:
name: Name of the new Field. String.
datatype: Datatype of the new field. String.
role: Role of the new field. String.
type: Type of the new field. String.
field_type: Type of the new field. String.
caption: Caption of the new field. String.

Returns:
Expand All @@ -284,12 +284,7 @@ def add_field(self, name, datatype, role, type, caption):
caption = name.replace('[', '').replace(']', '').title()

# Create the elements
column = ET.Element('column')
column.set('caption', caption)
column.set('datatype', datatype)
column.set('role', role)
column.set('type', type)
column.set('name', name)
column = xfile.create_column(caption, datatype, role, field_type, name)

self._datasourceTree.getroot().append(column)

Expand Down Expand Up @@ -319,8 +314,6 @@ def remove_field(self, field):
def calculations(self):
""" Returns all calculated fields.
"""
# TODO: There is a default [Number of Records] calculation.
# Should this be excluded so users can't meddle with it?
return {k: v for k, v in self.fields.items() if v.calculation is not None}

def add_calculation(self, caption, formula, datatype, role, type):
Expand Down
28 changes: 6 additions & 22 deletions tableaudocumentapi/field.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import functools
import xml.etree.ElementTree as ET
from functools import wraps

from tableaudocumentapi.property_decorators import argument_is_one_of

_ATTRIBUTES = [
'id', # Name of the field as specified in the file, usually surrounded by [ ]
Expand All @@ -24,21 +25,6 @@
]


def argument_is_one_of(*allowed_values):
def property_type_decorator(func):
@wraps(func)
def wrapper(self, value):
if value not in allowed_values:
error = "Invalid argument: {0}. {1} must be one of {2}."
msg = error.format(value, func.__name__, allowed_values)
raise ValueError(error)
return func(self, value)

return wrapper

return property_type_decorator


def _find_metadata_record(record, attrib):
element = record.find('.//{}'.format(attrib))
if element is None:
Expand All @@ -58,8 +44,6 @@ def __init__(self, column_xml=None, metadata_xml=None):
setattr(self, '_{}'.format(attrib), None)
self._worksheets = set()

self._xml = None

if column_xml is not None:
self._initialize_from_column_xml(column_xml)
self._xml = column_xml
Expand Down Expand Up @@ -225,17 +209,17 @@ def type(self):

@type.setter
@argument_is_one_of('quantitative', 'ordinal', 'nominal')
def type(self, type):
def type(self, field_type):
""" Set the type of a field

Args:
type: New type. String.
field_type: New type. String.

Returns:
Nothing.
"""
self._type = type
self._xml.set('type', type)
self._type = field_type
self._xml.set('type', field_type)

########################################
# Aliases getter and setter
Expand Down
15 changes: 15 additions & 0 deletions tableaudocumentapi/property_decorators.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
from functools import wraps

def argument_is_one_of(*allowed_values):
def property_type_decorator(func):
@wraps(func)
def wrapper(self, value):
if value not in allowed_values:
error = "Invalid argument: {0}. {1} must be one of {2}."
msg = error.format(value, func.__name__, allowed_values)
raise ValueError(error)
return func(self, value)

return wrapper

return property_type_decorator
10 changes: 10 additions & 0 deletions tableaudocumentapi/xfile.py
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,16 @@ def get_xml_from_archive(filename):
return xml_tree


def create_column(caption, datatype, role, field_type, name):
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This looks fine, but xfile is an odd place, I say toss it back in fields.py -- did @RussTheAerialist recommend it go here?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@RussTheAerialist wanted this as a column tag factory so it would read

column = xml_factory.create_column(caption, datatype, role, type_, name)

So I put it into xfile, since it's the closest thing to a dedicated XML class that I could find in this project. I'll put it back into fields.py

column = ET.Element('column')
column.set('caption', caption)
column.set('datatype', datatype)
column.set('role', role)
column.set('type', field_type)
column.set('name', name)
return column


def build_archive_file(archive_contents, zip_file):
for root_dir, _, files in os.walk(archive_contents):
relative_dir = os.path.relpath(root_dir, archive_contents)
Expand Down
59 changes: 30 additions & 29 deletions test/test_field_change.py
Original file line number Diff line number Diff line change
Expand Up @@ -63,11 +63,31 @@ def current_hash(self):
"""
return hash(ET.tostring(self.tds._datasourceTree.getroot()))

def check_state_change(self, should_change, msg, field_name):
""" Check whether the XML has changed and update the current state.

Args:
should_change: Whether the XML is supposed to have changed or not. Boolean.
msg: The message to be displayed in an error case, as key for the MESSAGES dict. String.
field_name: The field name that will be displayed in the error message. String.

Returns:
Nothing.
"""
new_state = self.current_hash()
compare_func = self.assertNotEqual if should_change else self.assertEqual
compare_func(
self.state,
new_state,
msg=MESSAGES[msg].format(field_name)
)
self.state = new_state

def test_change_values(self):
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

There are many similarities between the code in this test and the next one. Is there a way to extract the mostly common code and replace the differences with arguments to the new function?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Extracted code into a new function.

""" Test if the value changes of a field are reflected in the object and in the underlying XML structure.
"""
field_to_test = "[amount]"
state = self.current_hash()
self.state = self.current_hash()
# change all fields
for key, value in NEW_VALUES.items():
setattr(self.tds.fields[field_to_test], key, value)
Expand All @@ -78,19 +98,13 @@ def test_change_values(self):
msg=MESSAGES['test_change_values1'].format(key)
)
# the new value must be reflected in the xml
new_state = self.current_hash()
self.assertNotEqual(
state,
new_state,
msg=MESSAGES['test_change_values2'].format(key)
)
state = new_state
self.check_state_change(True, 'test_change_values2', key)

def test_change_values_fail(self):
""" Test if the value changes of a field are rejected if the wrong arguments are passed.
"""
field_to_test = "[amount]"
state = self.current_hash()
self.state = self.current_hash()
# change all fields
for key, value in WRONG_VALUES.items():

Expand All @@ -104,42 +118,29 @@ def test_change_values_fail(self):
value,
msg=MESSAGES['test_change_valuesFail1'].format(key)
)

# the new value must NOT be reflected in the xml
new_state = self.current_hash()
self.assertEqual(
state,
new_state,
msg=MESSAGES['test_change_valuesFail2'].format(key)
)
state = new_state
self.check_state_change(False, 'test_change_valuesFail2', key)

def test_remove_field(self):
""" Test if a Field can be removed.
"""
field_to_test = "[amount]"
state = self.current_hash()
self.state = self.current_hash()
# change all fields
field = self.tds.fields["[amount]"]
self.tds.remove_field(field)
self.assertNotEqual(state, self.current_hash())
self.assertNotEqual(self.state, self.current_hash())

def test_change_aliases(self):
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This one is also very similar to the two above that I previously commented on.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

See above.

""" Test if the alias changes of a field are reflected in the object and in the underlying XML structure.
"""
field_to_test = "[amount]"
state = self.current_hash()
self.state = self.current_hash()
# change all fields
for key, value in ALIASES.items():
self.tds.fields[field_to_test].add_alias(key, value)
# the new value must be reflected in the xml
new_state = self.current_hash()
self.assertNotEqual(
state,
new_state,
msg=MESSAGES['test_change_aliases1'].format(field_to_test)
)
state = new_state
self.check_state_change(True, 'test_change_aliases1', field_to_test)

# check whether all fields of ALIASES have been applied
self.assertEqual(
Expand All @@ -165,7 +166,7 @@ def test_calculation_base(self):
def test_calculation_change(self):
""" Test whether changing calculations of a field works.
"""
state = self.current_hash()
self.state = self.current_hash()
new_calc = '33 * 44'
fld_name = '[Calculation_357754699576291328]'
self.tds.calculations[fld_name].calculation = new_calc
Expand All @@ -175,7 +176,7 @@ def test_calculation_change(self):

# Check XML representation
new_state = self.current_hash()
self.assertNotEqual(state, new_state)
self.assertNotEqual(self.state, new_state)

def test_calculation_new(self):
""" Test if creating a new calculation works.
Expand Down