Skip to content

Commit

Permalink
code style and minor improvements, better test coverage
Browse files Browse the repository at this point in the history
  • Loading branch information
@matijakolaric
matijakolaric committed Dec 20, 2019
1 parent a426ec8 commit 7bbb8e9
Show file tree
Hide file tree
Showing 5 changed files with 224 additions and 56 deletions.
34 changes: 23 additions & 11 deletions music_metadata/edi/fields.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ def __init__(self, size, mandatory=False, *args, **kwargs):

def __set_name__(self, owner, name):
self._name = name
self._record = owner

def __get__(self, instance, owner=None):
if instance:
Expand All @@ -49,8 +50,10 @@ def verbose(self, value):
"""Return verbose (human-readable) value"""
return value

def to_html(self, value, label='', error=None):
def to_html(self, value, label=None, error=None):
"""Create HTML representation for EDI, used in syntax highlighting"""
if label is None:
label = self._name
if self:
edi_value = self.to_edi(value)
verbose_value = self.verbose(value)
Expand All @@ -73,6 +76,8 @@ def to_html(self, value, label='', error=None):

def to_dict(self, record, label=None, verbosity=1):
"""Create the dictionary with the value and additional data."""
if label is None:
label = self._name
value = getattr(record, label or self._name)
valid = record.valid or label not in record.errors
if value is None and valid and verbosity <= 1:
Expand Down Expand Up @@ -105,8 +110,10 @@ def __set__(self, instance, value):
try:
value = int(value)
except ValueError:
super().__set__(instance, value)
raise RecordError(f'Value "{value}" is not numeric')
if not 0 <= value < 10 ** self._size:
super().__set__(instance, value)
raise FieldError(
f'Not between 0 "{value}" and "{10 ** self._size - 1}"')
super().__set__(instance, value)
Expand All @@ -126,8 +133,9 @@ def __init__(self, size, constant=None, *args, **kwargs):
if len(constant) == size:
self._constant = constant
else:
length = len(constant)
raise AttributeError(
f'Value "{value}" is not {size} characters long.')
f'Value "{ length }" is not { size } characters long.')
else:
self._constant = ' ' * size
super().__init__(size, *args, **kwargs)
Expand Down Expand Up @@ -155,8 +163,8 @@ def __set__(self, instance, value):
value = value.strip()
if value and value not in self._choices.keys():
super().__set__(instance, value)
raise FieldError('Value must be one of '
f'''\"{'", "'.join(self._edi_keys)}\"''')
keys = ', '.join(self._edi_keys)
raise FieldError(f'Value must be one of "{ keys }"')
super().__set__(instance, value)

def verbose(self, value):
Expand Down Expand Up @@ -188,13 +196,14 @@ def __set__(self, instance, value):
instance.__dict__[self._name] = None
return
value = dict(
(('Y', True), ('N', False), ('U', None), (' ', None))).get(value,
value)
(('Y', True), ('N', False), ('U', None), (' ', None))
).get(value, value)
super().__set__(instance, value)

def to_edi(self, value):
value = dict(((True, 'Y'), (False, 'N'),
(None, 'U' if self._mandatory else ' '))).get(value, ' ')
value = dict((
(True, 'Y'), (False, 'N'), (None, 'U' if self._mandatory else ' ')
)).get(value, ' ')
return value


Expand All @@ -210,10 +219,13 @@ def __init__(self, *args, **kwargs):
self._edi_keys = ('Y', 'N')

def __set__(self, instance, value):
value = dict((('Y', True), ('N', False), (' ', None))).get(value,
value)
value = dict((
('Y', True), ('N', False), (' ', None)
)).get(value, value)
super().__set__(instance, value)

def to_edi(self, value):
value = dict(((True, 'Y'), (False, 'N'), (None, ' '))).get(value, ' ')
value = dict((
(True, 'Y'), (False, 'N'), (None, ' ')
)).get(value, ' ')
return value
48 changes: 26 additions & 22 deletions music_metadata/edi/file.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ def get_header(self):
f'{self.sequence}')
self.errors.append(e)
self._header.error('group_code', e)
self.get_file().valid = False
self.valid &= self._header.valid
return self._header
return None
Expand All @@ -77,6 +78,7 @@ def get_trailer(self):
f'{self.sequence}')
self.errors.append(e)
self._trailer.error('group_code', e)
self.get_file().valid = False
return self._trailer
return None

Expand All @@ -88,13 +90,13 @@ def get_transaction_class(self):

def get_transactions(self):
sequence = 0
pattern = re.compile(r'(^{0}.*?(?=^GRT|^{0}))'.format(self.type),
re.S | re.M)
pattern = re.compile(
r'(^{0}.*?(?=^GRT|^{0}))'.format(self.type), re.S | re.M)
try:
for transaction_lines in re.findall(pattern, self.lines):
transaction_class = self.get_transaction_class()
transaction = transaction_class(self.type, transaction_lines,
sequence)
transaction = transaction_class(
self.type, transaction_lines, sequence)
for error in transaction.errors:
if isinstance(error, FileError):
self.valid = False
Expand All @@ -111,17 +113,19 @@ def get_transactions(self):
if self.transaction_count != trailer.transaction_count:
self.valid = False
self.get_file().valid = False
e = FileError(f'Wrong transaction count in GRT: '
f'{trailer.transaction_count}, counted '
f'{self.transaction_count}')
e = FileError(
f'Wrong transaction count in GRT: '
f'{trailer.transaction_count}, counted '
f'{self.transaction_count}')
self.errors.append(e)
trailer.error('transaction_count', e)
if self.record_count != trailer.record_count:
self.valid = False
self.get_file().valid = False
e = FileError(f'Wrong record count in GRT: '
f'{trailer.record_count}, counted '
f'{self.record_count}')
e = FileError(
f'Wrong record count in GRT: '
f'{trailer.record_count}, counted '
f'{self.record_count}')
self.errors.append(e)
trailer.error('record_count', e)
except FileError as e:
Expand Down Expand Up @@ -221,27 +225,30 @@ def get_groups(self):

if expected_sequence != trailer.group_count:
self.valid = False
e = FileError('Wrong group count in TRL: '
f'{trailer.group_count}, '
f'counted {expected_sequence}')
e = FileError(
'Wrong group count in TRL: '
f'{trailer.group_count}, '
f'counted {expected_sequence}')
self.file_errors.append(e)
trailer.error('group_count', e)

self.transaction_count = transaction_count
if transaction_count != trailer.transaction_count:
self.valid = False
e = FileError('Wrong transaction count in TRL: '
f'{trailer.transaction_count}, '
f'GRTs say {transaction_count}')
e = FileError(
'Wrong transaction count in TRL: '
f'{trailer.transaction_count}, '
f'GRTs say {transaction_count}')
self.file_errors.append(e)
trailer.error('transaction_count', e)

self.record_count = record_count
if record_count != trailer.record_count:
self.valid = False
e = FileError('Wrong record count in TRL: '
f'{trailer.record_count}, '
f'GRTs say {record_count}')
e = FileError(
'Wrong record count in TRL: '
f'{trailer.record_count}, '
f'GRTs say {record_count}')
self.file_errors.append(e)
trailer.error('record_count', e)

Expand All @@ -250,6 +257,3 @@ def list_groups(self):

def get_encoding_from_header(self):
return 'latin1'

def to_dict(self):
raise Exception('CWR not loaded!')
43 changes: 25 additions & 18 deletions music_metadata/edi/records.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,11 @@ def __init__(self, line=None, sequence=None):
else:
raise FileError(f'Record too short: {line}')

def __setattr__(self, key, value):
super().__setattr__(key, value)
if key == 'record_type' and self.type is None:
self.type = key

def to_edi(self):
output = ''
for label, field in self._fields.items():
Expand All @@ -60,7 +65,8 @@ def __str__(self):
def warning(self, field, error):
"""Add an error, do not invalidate."""
if field and field not in self.labels:
raise AttributeError(f'No such field {field} in {self.labels}')
labels = ', '.join(self.labels)
raise AttributeError(f'No such field { field } in { labels }')
self.errors[field] = error

def error(self, field, error):
Expand All @@ -86,12 +92,12 @@ def split_into_fields(self):
pos += field._size
value = self.line[start:end]
if start < actual_length < end:
self.warning(label, FieldError('Field truncated'))
self.warning(label, FieldWarning('Field truncated'))
elif end > actual_length:
if field._mandatory:
self.error(label, RecordError('Mandatory field missing'))
else:
self.warning(label, FieldError(
self.warning(label, FieldWarning(
'Field missing at the end of the line.'))
try:
setattr(self, label, value)
Expand All @@ -117,19 +123,18 @@ def labels(self):
return self.get_fields().keys()

def to_html(self):
classes = f'record {self.type.lower()}'
classes = f'record { self.type.lower() }'
if not self.valid:
classes += ' invalid'
output = f'<span class="{classes}">'
output = f'<span class="{ classes }">'
for field in self.fields:
value = getattr(self, field._name)
s = field.to_html(value, f'{field._name}',
self.errors.get(field._name))
s = field.to_html(
value, f'{ field._name }', self.errors.get(field._name))
output += s
else:
output += EdiField.to_html(
None, self.rest, label='',
error=self.errors.get(None))
None, self.rest, label='', error=self.errors.get(None))
output += '</span>'
return output

Expand All @@ -147,15 +152,17 @@ def validate_sequences(self, transaction_sequence, record_sequence):
"""This is really a file-level validation."""

if self.transaction_sequence_number != transaction_sequence:
e = FileError(f'Wrong transaction sequence '
f'{self.transaction_sequence_number}, should be '
f'{transaction_sequence}')
e = FileError(
f'Wrong transaction sequence '
f'{ self.transaction_sequence_number }, should be '
f'{ transaction_sequence }')
self.error('transaction_sequence_number', e)

if self.record_sequence_number != record_sequence:
e = FileError(f'Wrong transaction sequence '
f'{self.record_sequence_number}, should be '
f'{record_sequence}')
e = FileError(
f'Wrong transaction sequence '
f'{ self.record_sequence_number }, should be '
f'{ record_sequence }')
self.error('record_sequence_number', e)

def validate(self):
Expand All @@ -168,9 +175,9 @@ def to_dict(self, verbosity=1):

# first three fields can be skipped
if label in [
'record_type',
'transaction_sequence_number',
'record_sequence_number'
'record_type',
'transaction_sequence_number',
'record_sequence_number'
] and label not in self.errors.keys():
continue

Expand Down

0 comments on commit 7bbb8e9

Please sign in to comment.