Skip to content

Sourcery refactored master branch#1

Open
sourcery-ai[bot] wants to merge 1 commit into
masterfrom
sourcery/master
Open

Sourcery refactored master branch#1
sourcery-ai[bot] wants to merge 1 commit into
masterfrom
sourcery/master

Conversation

@sourcery-ai
Copy link
Copy Markdown

@sourcery-ai sourcery-ai Bot commented Nov 7, 2022

Branch master refactored by Sourcery.

If you're happy with these changes, merge this Pull Request using the Squash and merge strategy.

See our documentation here.

Run Sourcery locally

Reduce the feedback loop during development by using the Sourcery editor plugin:

Review changes via command line

To manually merge these changes, make sure you're on the master branch, then run:

git fetch origin sourcery/master
git merge --ff-only FETCH_HEAD
git reset HEAD^

Help us improve this pull request!

@sourcery-ai sourcery-ai Bot requested a review from bruvio November 7, 2022 08:45
Comment thread fitparse/base.py
Comment on lines -45 to +50
raise FitParseError("No such dev_data_index=%s found" % (dev_data_index))
raise FitParseError(f"No such dev_data_index={dev_data_index} found")

warnings.warn(
"Dev type for dev_data_index=%s missing. Adding dummy dev type." % (dev_data_index)
f"Dev type for dev_data_index={dev_data_index} missing. Adding dummy dev type."
)

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Function DeveloperDataMixin._append_dev_field_description refactored with the following changes:

Comment thread fitparse/base.py
Comment on lines -65 to +81
field_name = message.get_raw_value('field_name') or "unnamed_dev_field_%s" % field_def_num
field_name = (
message.get_raw_value('field_name')
or f"unnamed_dev_field_{field_def_num}"
)

units = message.get_raw_value("units")
native_field_num = message.get_raw_value('native_field_num')

if dev_data_index not in self.dev_types:
if self.check_developer_data:
raise FitParseError("No such dev_data_index=%s found" % (dev_data_index))
raise FitParseError(f"No such dev_data_index={dev_data_index} found")

warnings.warn(
"Dev type for dev_data_index=%s missing. Adding dummy dev type." % (dev_data_index)
f"Dev type for dev_data_index={dev_data_index} missing. Adding dummy dev type."
)

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Function DeveloperDataMixin.add_dev_field_description refactored with the following changes:

Comment thread fitparse/base.py
Comment on lines -98 to +106
"Dev type for dev_data_index=%s missing. Adding dummy dev type." % (dev_data_index)
f"Dev type for dev_data_index={dev_data_index} missing. Adding dummy dev type."
)

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Function DeveloperDataMixin.get_dev_type refactored with the following changes:

Comment thread fitparse/base.py
Comment on lines -173 to +180
raise FitParseError("Invalid struct format: %s" % fmt_with_endian)
raise FitParseError(f"Invalid struct format: {fmt_with_endian}")
Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Function FitFileDecoder._read_struct refactored with the following changes:

Comment thread fitparse/base.py
Comment on lines -190 to +199
raise FitCRCError('CRC Mismatch [computed: {}, read: {}]'.format(
Crc.format(crc_computed), Crc.format(crc_read)))
raise FitCRCError(
f'CRC Mismatch [computed: {Crc.format(crc_computed)}, read: {Crc.format(crc_read)}]'
)
Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Function FitFileDecoder._read_and_assert_crc refactored with the following changes:

Comment thread fitparse/records.py
Comment on lines -176 to +178
if self.field:
if name in (self.field.name, self.field.def_num):
return True
if self.parent_field:
if name in (self.parent_field.name, self.parent_field.def_num):
return True
if self.field_def:
if name == self.field_def.def_num:
return True
return False
if self.field and name in (self.field.name, self.field.def_num):
return True
if self.parent_field and name in (
self.parent_field.name,
self.parent_field.def_num,
):
return True
return bool(self.field_def and name == self.field_def.def_num)
Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Function FieldData.is_named refactored with the following changes:

Comment thread fitparse/records.py
Comment on lines -219 to +222
return '<FieldData: %s: %s%s, def num: %d, type: %s (%s), raw value: %s>' % (
self.name, self.value, ' [%s]' % self.units if self.units else '',
self.def_num, self.type.name, self.base_type.name, self.raw_value,
return (
'<FieldData: %s: %s%s, def num: %d, type: %s (%s), raw value: %s>'
% (
self.name,
self.value,
f' [{self.units}]' if self.units else '',
self.def_num,
self.type.name,
self.base_type.name,
self.raw_value,
)
Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Function FieldData.__repr__ refactored with the following changes:

Comment thread fitparse/records.py
Comment on lines -225 to +226
return '{}: {}{}'.format(
self.name, self.value, ' [%s]' % self.units if self.units else '',
)
return f"{self.name}: {self.value}{f' [{self.units}]' if self.units else ''}"
Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Function FieldData.__str__ refactored with the following changes:

Comment thread fitparse/records.py
Comment on lines -350 to +349
return '<{} {}>'.format(self.__class__.__name__, self.value or "-")
return f'<{self.__class__.__name__} {self.value or "-"}>'
Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Function Crc.__repr__ refactored with the following changes:

Comment thread fitparse/utils.py
method_name = method_name.replace(
replace_from, '%s' % replace_to,
)
method_name = method_name.replace(replace_from, f'{replace_to}')
Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Function scrub_method_name refactored with the following changes:

@sourcery-ai
Copy link
Copy Markdown
Author

sourcery-ai Bot commented Nov 7, 2022

Sourcery Code Quality Report

❌  Merging this PR will decrease code quality in the affected files by 0.12%.

Quality metrics Before After Change
Complexity 11.21 🙂 10.65 🙂 -0.56 👍
Method Length 59.48 ⭐ 59.62 ⭐ 0.14 👎
Working memory 8.32 🙂 8.49 🙂 0.17 👎
Quality 65.47% 🙂 65.35% 🙂 -0.12% 👎
Other metrics Before After Change
Lines 2206 2219 13
Changed files Quality Before Quality After Quality Change
fitparse/base.py 64.23% 🙂 63.82% 🙂 -0.41% 👎
fitparse/processors.py 87.85% ⭐ 87.83% ⭐ -0.02% 👎
fitparse/records.py 88.47% ⭐ 88.49% ⭐ 0.02% 👍
fitparse/utils.py 83.72% ⭐ 83.72% ⭐ 0.00%
scripts/fitdump 44.31% 😞 44.30% 😞 -0.01% 👎
scripts/generate_profile.py 58.81% 🙂 57.74% 🙂 -1.07% 👎
scripts/unit_tool.py 64.97% 🙂 67.40% 🙂 2.43% 👍
tests/test.py 62.64% 🙂 63.99% 🙂 1.35% 👍

Here are some functions in these files that still need a tune-up:

File Function Complexity Length Working Memory Quality Recommendation
scripts/generate_profile.py parse_messages 56 ⛔ 710 ⛔ 26 ⛔ 3.94% ⛔ Refactor to reduce nesting. Try splitting into smaller methods. Extract out complex expressions
scripts/fitdump generate_gpx 41 ⛔ 288 ⛔ 16 ⛔ 16.55% ⛔ Refactor to reduce nesting. Try splitting into smaller methods. Extract out complex expressions
fitparse/base.py FitFileDecoder._parse_data_message_components 27 😞 302 ⛔ 17 ⛔ 20.84% ⛔ Refactor to reduce nesting. Try splitting into smaller methods. Extract out complex expressions
tests/test.py FitFileTestCase._csv_test_helper 42 ⛔ 231 ⛔ 12 😞 24.32% ⛔ Refactor to reduce nesting. Try splitting into smaller methods. Extract out complex expressions
scripts/generate_profile.py main 14 🙂 347 ⛔ 17 ⛔ 28.73% 😞 Try splitting into smaller methods. Extract out complex expressions

Legend and Explanation

The emojis denote the absolute quality of the code:

  • ⭐ excellent
  • 🙂 good
  • 😞 poor
  • ⛔ very poor

The 👍 and 👎 indicate whether the quality has improved or gotten worse with this pull request.


Please see our documentation here for details on how these metrics are calculated.

We are actively working on this report - lots more documentation and extra metrics to come!

Help us improve this quality report!

Comment thread scripts/fitdump
Comment on lines -34 to +36
epilog='python-fitparse version %s' % fitparse.__version__,
epilog=f'python-fitparse version {fitparse.__version__}',
)

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Function parse_args refactored with the following changes:


def header(header, indent=0):
return '{}# {}'.format(' ' * indent, (' %s ' % header).center(78 - indent, '*'))
return f"{' ' * indent}# {f' {header} '.center(78 - indent, '*')}"
Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Function header refactored with the following changes:

Comment on lines -38 to +39
PROFILE_HEADER_FIRST_PART = "{}\n{}".format(
header('BEGIN AUTOMATICALLY GENERATED FIT PROFILE'),
header('DO NOT EDIT THIS FILE'),
)
PROFILE_HEADER_FIRST_PART = f"{header('BEGIN AUTOMATICALLY GENERATED FIT PROFILE')}\n{header('DO NOT EDIT THIS FILE')}"

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Lines 38-41 refactored with the following changes:

Comment on lines -95 to +93
return "BASE_TYPES[{}], # {}".format(BASE_TYPES[name], name)
return f"BASE_TYPES[{BASE_TYPES[name]}], # {name}"
Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Function render_type refactored with the following changes:

Comment on lines -124 to +122
s += " '{}': {},\n".format(type.name, indent(type))
s += f" '{type.name}': {indent(type)},\n"
Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Function TypeList.__str__ refactored with the following changes:

Comment on lines -361 to +358
if isinstance(value, float):
if value.is_integer():
value = int(value)
if isinstance(value, float) and value.is_integer():
value = int(value)

values.append(value)

if all(v is None for v in values):
continue

parsed_values.append(values)
if any(v is not None for v in values):
parsed_values.append(values)
Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Function parse_spreadsheet refactored with the following changes:

Comment on lines -392 to +385
if value.name and value.value is not None:
# Don't add ignore keyed types
if "{}:{}".format(type.name, value.name) not in IGNORE_TYPE_VALUES:
type.values.append(value)
if (
value.name
and value.value is not None
and f"{type.name}:{value.name}" not in IGNORE_TYPE_VALUES
):
type.values.append(value)
Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Function parse_types refactored with the following changes:

This removes the following comments ( why? ):

# Don't add ignore keyed types

Comment on lines -405 to +395
if isinstance(o, bytes):
return o.decode()
return o
return o.decode() if isinstance(o, bytes) else o
Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Function maybe_decode refactored with the following changes:

Comment on lines -534 to +526
version_match = re.search(
if version_match := re.search(
r'Profile Version.+?(\d+\.?\d*).*',
archive.open('c/fit.h').read().decode(),
)
if version_match:
profile_version = ("%f" % float(version_match.group(1))).rstrip('0').ljust(4, '0')
):
profile_version = ("%f" % float(version_match[1])).rstrip('0').ljust(4, '0')
Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Function get_xls_and_version_from_zip refactored with the following changes:

Comment on lines -548 to +544
if output_py_path and os.path.exists(output_py_path):
if not open(output_py_path).read().strip().startswith(PROFILE_HEADER_FIRST_PART):
print("Python file doesn't begin with appropriate header. Exiting.")
sys.exit(1)
if (
output_py_path
and os.path.exists(output_py_path)
and not open(output_py_path)
.read()
.strip()
.startswith(PROFILE_HEADER_FIRST_PART)
):
print("Python file doesn't begin with appropriate header. Exiting.")
sys.exit(1)
Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Function main refactored with the following changes:

Comment on lines -621 to +627
print("Usage: %s <FitSDK.zip | Profile.xls> [profile.py]" % os.path.basename(__file__))
print(
f"Usage: {os.path.basename(__file__)} <FitSDK.zip | Profile.xls> [profile.py]"
)

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Lines 621-621 refactored with the following changes:

Comment thread scripts/unit_tool.py
Comment on lines -20 to +27
unit_value = unit_value.strip()
if unit_value:
if unit_value := unit_value.strip():
# Deal with comma separated components
unit_values = [v.strip() for v in unit_value.split(',')]
all_unit_values.extend(unit_values)

print('In Profile.xls:')
for unit_value in sorted(set(all_unit_values)):
print(' * %s' % unit_value)
print(f' * {unit_value}')
Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Function do_profile_xls refactored with the following changes:

Comment thread scripts/unit_tool.py
Comment on lines -37 to +53
for component in field.components:
unit_values.append(component.units)
unit_values.extend(component.units for component in field.components)
if field.subfields:
for subfield in field.subfields:
unit_values.append(subfield.units)
if subfield.components:
for component in subfield.components:
unit_values.append(component.units)

unit_values.extend(component.units for component in subfield.components)
unit_values = filter(None, unit_values)

print('In fitparse/profile.py:')
for unit_value in sorted(set(unit_values)):
print(' * {} [{}]'.format(
unit_value,
scrub_method_name('process_units_%s' % unit_value, convert_units=True)
))
print(
' * {} [{}]'.format(
unit_value,
scrub_method_name(
f'process_units_{unit_value}', convert_units=True
),
)
)
Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Function do_fitparse_profile refactored with the following changes:

Comment thread tests/test.py


def generate_messages(mesg_num, local_mesg_num, field_defs, endian='<', data=None):
mesgs = []
Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Function generate_messages refactored with the following changes:

Comment thread tests/test.py
Comment on lines -64 to +63
file_data = header + pack('<' + Crc.FMT, Crc.calculate(header)) + fit_data
return file_data + pack('<' + Crc.FMT, Crc.calculate(file_data))
file_data = header + pack(f'<{Crc.FMT}', Crc.calculate(header)) + fit_data
return file_data + pack(f'<{Crc.FMT}', Crc.calculate(file_data))
Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Function generate_fitfile refactored with the following changes:

Comment thread tests/test.py
Comment on lines -108 to +133
# TODO: abstract CSV parsing
csv_fp = open(testfile('compressed-speed-distance-records.csv'))
csv_file = csv.reader(csv_fp)
next(csv_file) # Consume header
with open(testfile('compressed-speed-distance-records.csv')) as csv_fp:
csv_file = csv.reader(csv_fp)
next(csv_file) # Consume header

f = FitFile(testfile('compressed-speed-distance.fit'))
f.parse()
f = FitFile(testfile('compressed-speed-distance.fit'))
f.parse()

records = f.get_messages(name='record')
empty_record = next(records) # Skip empty record for now (sets timestamp via header)
records = f.get_messages(name='record')
empty_record = next(records) # Skip empty record for now (sets timestamp via header)

# File's timestamp record is < 0x10000000, so field returns seconds
self.assertEqual(empty_record.get_value('timestamp'), 17217864)
# File's timestamp record is < 0x10000000, so field returns seconds
self.assertEqual(empty_record.get_value('timestamp'), 17217864)

# TODO: update using local_timestamp as offset, since we have this value as 2012 date
# TODO: update using local_timestamp as offset, since we have this value as 2012 date

for count, (record, (timestamp, heartrate, speed, distance, cadence)) in enumerate(zip(records, csv_file)):
# No fancy datetime stuff, since timestamp record is < 0x10000000
fit_ts = record.get_value('timestamp')
self.assertIsInstance(fit_ts, int)
self.assertLess(fit_ts, 0x10000000)
self.assertEqual(fit_ts, int(timestamp))
for count, (record, (timestamp, heartrate, speed, distance, cadence)) in enumerate(zip(records, csv_file)):
# No fancy datetime stuff, since timestamp record is < 0x10000000
fit_ts = record.get_value('timestamp')
self.assertIsInstance(fit_ts, int)
self.assertLess(fit_ts, 0x10000000)
self.assertEqual(fit_ts, int(timestamp))

self.assertEqual(record.get_value('heart_rate'), int(heartrate))
self.assertEqual(record.get_value('cadence'), int(cadence) if cadence != 'null' else None)
self.assertAlmostEqual(record.get_value('speed'), float(speed))
self.assertAlmostEqual(record.get_value('distance'), float(distance))
self.assertEqual(record.get_value('heart_rate'), int(heartrate))
self.assertEqual(record.get_value('cadence'), int(cadence) if cadence != 'null' else None)
self.assertAlmostEqual(record.get_value('speed'), float(speed))
self.assertAlmostEqual(record.get_value('distance'), float(distance))

self.assertEqual(count, 753) # TODO: confirm size(records) = size(csv)
csv_fp.close()
self.assertEqual(count, 753) # TODO: confirm size(records) = size(csv)
Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Function FitFileTestCase.test_component_field_accumulaters refactored with the following changes:

This removes the following comments ( why? ):

# TODO: abstract CSV parsing

Comment thread tests/test.py
Comment on lines -255 to +311
csv_fp = open(testfile(csv_file))
csv_messages = csv.reader(csv_fp)
field_names = next(csv_messages) # Consume header

f = FitFile(testfile(fit_file))
messages = f.get_messages(name='record')

# For fixups
last_valid_lat, last_valid_long = None, None

for message, csv_message in zip(messages, csv_messages):
for csv_index, field_name in enumerate(field_names):
fit_value, csv_value = message.get_value(field_name), csv_message[csv_index]
if field_name == 'timestamp':
# Adjust GMT to PDT and format
fit_value = (fit_value - datetime.timedelta(hours=7)).strftime("%a %b %d %H:%M:%S PDT %Y")

# Track last valid lat/longs
if field_name == 'position_lat':
if fit_value is not None:
with open(testfile(csv_file)) as csv_fp:
csv_messages = csv.reader(csv_fp)
field_names = next(csv_messages) # Consume header

f = FitFile(testfile(fit_file))
messages = f.get_messages(name='record')

# For fixups
last_valid_lat, last_valid_long = None, None

for message, csv_message in zip(messages, csv_messages):
for csv_index, field_name in enumerate(field_names):
fit_value, csv_value = message.get_value(field_name), csv_message[csv_index]
if field_name == 'timestamp':
# Adjust GMT to PDT and format
fit_value = (fit_value - datetime.timedelta(hours=7)).strftime("%a %b %d %H:%M:%S PDT %Y")

# Track last valid lat/longs
if field_name == 'position_lat' and fit_value is not None:
last_valid_lat = fit_value
if field_name == 'position_long':
if fit_value is not None:
if field_name == 'position_long' and fit_value is not None:
last_valid_long = fit_value

# ANT FIT SDK Dump tool does a bad job of logging invalids, so fix them
if fit_value is None:
# ANT FIT SDK Dump tool cadence reports invalid as 0
if field_name == 'cadence' and csv_value == '0':
# ANT FIT SDK Dump tool does a bad job of logging invalids, so fix them
if fit_value is None:
# ANT FIT SDK Dump tool cadence reports invalid as 0
if field_name == 'cadence' and csv_value == '0':
csv_value = None
# ANT FIT SDK Dump tool invalid lat/lng reports as last valid
if field_name == 'position_lat':
fit_value = last_valid_lat
if field_name == 'position_long':
fit_value = last_valid_long

if isinstance(fit_value, int):
csv_value = int(fit_value)
if csv_value == '':
csv_value = None
# ANT FIT SDK Dump tool invalid lat/lng reports as last valid
if field_name == 'position_lat':
fit_value = last_valid_lat
if field_name == 'position_long':
fit_value = last_valid_long

if isinstance(fit_value, int):
csv_value = int(fit_value)
if csv_value == '':
csv_value = None

if isinstance(fit_value, float):
# Float comparison
self.assertAlmostEqual(fit_value, float(csv_value))
else:
self.assertEqual(fit_value, csv_value,
msg="For {}, FIT value '{}' did not match CSV value '{}'".format(field_name, fit_value, csv_value))

try:
next(messages)
self.fail(".FIT file had more than csv file")
except StopIteration:
pass

try:
next(csv_messages)
self.fail(".CSV file had more messages than .FIT file")
except StopIteration:
pass

csv_fp.close()
if isinstance(fit_value, float):
# Float comparison
self.assertAlmostEqual(fit_value, float(csv_value))
else:
self.assertEqual(
fit_value,
csv_value,
msg=f"For {field_name}, FIT value '{fit_value}' did not match CSV value '{csv_value}'",
)


try:
next(messages)
self.fail(".FIT file had more than csv file")
except StopIteration:
pass

try:
next(csv_messages)
self.fail(".CSV file had more messages than .FIT file")
except StopIteration:
pass
Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Function FitFileTestCase._csv_test_helper refactored with the following changes:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

0 participants