Skip to content

fix(python): write loadable class filenames in to_config - #4163

Merged
ryanmelt merged 3 commits into
fix/packet-item-to-config-statesfrom
fix/python-to-config-class-filenames
Oct 28, 2026
Merged

ryanmelt merged 3 commits into
fix/packet-item-to-config-statesfrom
fix/python-to-config-class-filenames

Conversation

@jmthomas

Copy link
Copy Markdown
Member

Stacked on #4154.

What changed

Python to_config wrote a bare class name (e.g. WatermarkProcessor) wherever a config keyword names a class. The Python parser loads these by file path (openc3/processors/watermark_processor.py), so the output didn't load back in. A new class_to_filename() helper in utilities/string.py builds the path from the class's module, the inverse of filename_to_module / filename_to_class_name. It's now used for:

Keyword File Before
PROCESSOR processor.py, watermark_processor.py, statistics_processor.py failed to load
READ/WRITE_CONVERSION conversion.py, processor_conversion.py, unix_time_conversion.py (and the 7 conversions that inherit to_config) failed to load
LIMITS_RESPONSE limits_response.py failed to load
VALIDATOR packet.py failed to load (the parser has no class-name fallback)
ACCESSOR, SUBPACKETIZER packet.py loaded only for built-in classes, through a deprecated fallback

Because the path comes from the class's module, custom classes in a target's lib load back in too.

Other bugs fixed along the way, each of which stopped to_config or as_json working on its own:

  • WatermarkProcessor.as_json used self.__class__.__name__.name and raised AttributeError.
  • IpWriteConversion.to_config(self) was missing the read_or_write parameter, so every call raised TypeError.
  • PacketItem.to_config added limits.response.to_config to the string without calling it, which raised TypeError.
  • UnixTimeConversion.to_config wrote a literal None when there's no microseconds item, and the parser would read that as an item name. It's now left out, which is what Ruby does.

The Ruby side already writes filenames and needs no change.

Testing

  • test_to_config_output_parses_back_to_the_same_classes (in test_packet_config.py) writes packets that use every keyword above with PacketConfig.to_config, loads the files back in, and checks every class and its parameters match. It fails on the old code.
  • Added WatermarkProcessor.as_json test; updated two assertions that expected the old class-name output.
  • A one-off script round-tripped all 16 built-in Python conversions plus the other keywords: all pass with the fix, and most failed without it.
  • Full Python suite: 3159 passed. ruff check / ruff format --check are clean.

Not included

PacketItem.as_json (packet_item.py:500) stores the limits response object itself in config["limits"]["response"], so the JSON for any item with a limits response can't be serialized. Ruby stores response.to_s. It's a separate bug, left for a follow-up.

🤖 Generated with Claude Code

- Write the module path, not the class name, for PROCESSOR,
  READ/WRITE_CONVERSION, LIMITS_RESPONSE, ACCESSOR, VALIDATOR and
  SUBPACKETIZER via a new class_to_filename helper, so output loads back
- Fix WatermarkProcessor.as_json, which raised AttributeError
- Fix IpWriteConversion.to_config missing its read_or_write argument and
  PacketItem.to_config not calling limits.response.to_config()
- Omit an unset microseconds item in UnixTimeConversion.to_config
  instead of writing a literal None

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Copilot AI balanced review requested due to automatic review settings October 27, 2026 04:00
@coderabbitai

coderabbitai Bot commented Oct 27, 2026 •

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

Run configuration
  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: defaults
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 05107808-179d-4a05-9573-60737673a48f

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

Copilot AI left a comment

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.

Copilot was unable to review this pull request because the user who requested the review has insufficient remaining quota.

An organization owner can choose the repository organization as the billing source for reviews requested by members with a paid Copilot license. Learn about Copilot code review billing.

@codecov

codecov Bot commented Oct 27, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 80.64%. Comparing base (3767540) to head (5cd15fc).

Additional details and impacted files
@@                         Coverage Diff                          @@
##           fix/packet-item-to-config-states    #4163      +/-   ##
====================================================================
+ Coverage                             80.59%   80.64%   +0.05%     
====================================================================
  Files                                   913      915       +2     
  Lines                                 71062    71095      +33     
  Branches                               2745     2745              
====================================================================
+ Hits                                  57270    57335      +65     
+ Misses                                13119    13090      -29     
+ Partials                                673      670       -3     
Flag Coverage Δ
frontend 68.44% <ø> (+0.04%) ⬆️
python 79.22% <ø> (+0.11%) ⬆️
ruby-api 85.22% <ø> (+0.07%) ⬆️
ruby-backend 86.60% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@jmthomas
jmthomas requested a review from ryanmelt October 27, 2026 04:41
@sonarqubecloud

Copy link
Copy Markdown

@ryanmelt
ryanmelt merged commit 6d10e03 into fix/packet-item-to-config-states Oct 28, 2026
43 of 44 checks passed
@ryanmelt
ryanmelt deleted the fix/python-to-config-class-filenames branch October 28, 2026 17:57
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.

3 participants