Loosen instrument naming requirement for febus files #525
Conversation
WalkthroughRemoves the top-level "fa" prefix check in _get_febus_version_str and initializes is_febus to True. Retains per-instrument checks for "Source" keys and required attributes, deriving version from Source Version. Adds an inline comment referencing #525. No public API or signatures changed. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Caller
participant Utils as _get_febus_version_str
participant Instruments as Instruments (dict)
participant Sources as Sources (attrs)
Caller->>Utils: call with dataset/attrs
note right of Utils: Previous top-level key check (\"fa\" prefix) removed
Utils->>Instruments: iterate instrument keys
loop For each instrument
Utils->>Sources: verify keys start with "Source"
alt Required attrs present
Utils->>Sources: read "Version"
else Missing/invalid attrs
Utils-->>Caller: return "" (empty string)
end
end
Utils-->>Caller: return derived version string
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 💡 Knowledge Base configuration:
You can enable these sources in your CodeRabbit configuration. 📒 Files selected for processing (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (15)
🔇 Additional comments (3)
✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #525 +/- ##
=======================================
Coverage 99.87% 99.87%
=======================================
Files 122 122
Lines 9938 9938
=======================================
Hits 9926 9926
Misses 12 12
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Description
Previously, part of the format check for Febus files looked at the instrument name. All the examples I had seen started with "fa" but it turns out this isn't always true. Since there are other strong checks, like the presence of "Source_X" datasets and specific attributes on each of these, I have simply removed this part of the febus format check.
Checklist
I have (if applicable):
Summary by CodeRabbit