feat: add France (FR) market support#132
Merged
Merged
Conversation
Adds "FR" to CountryCode alongside NL/BE. France is served through the same 'x-country' header mechanism already used for Belgium, via a new shared country_prices() helper (be_prices()/fr_prices() are now thin wrappers), and MarketPrices.from_be_dict() is generalized into from_country_dict(data, country). Also fixes two bugs surfaced by live-testing against a real France account: - Price.__init__ crashed with `TypeError: unsupported operand type(s) for +: 'float' and 'NoneType'` because France's public marketPrices response leaves marketPriceTax/sourcingMarkupPrice/energyTaxPrice as null (BE always returns numeric values for these). They now default to 0.0 when absent. - The 'x-country' marketPrices query never sent a $resolution argument at all, so it silently always returned the backend's default hourly data regardless of the configured resolution. This affected BE too, just never surfaced. country_prices()/be_prices()/fr_prices() now accept and forward a resolution parameter (default PT15M).
Handleiding voor reviewersVoegt ondersteuning voor de Franse (FR) markt toe via het bestaande x-country-mechanisme, generaliseert BE-specifieke marktprijs-parsing en -ophaling naar land-agnostische helpers, en verhelpt de verwerking van null-waarden voor belasting/opslagvelden en ontbrekende resolution in marketPrices-queries. Sequentiediagram voor country_prices marketPrices-query met resolution en FR-ondersteuningsequenceDiagram
actor User
participant FrankEnergieClient as FrankEnergie
participant GraphQLAPI
participant MarketPrices
User->>FrankEnergieClient: country_prices(country, start_date, end_date, resolution)
FrankEnergieClient->>FrankEnergieClient: FrankEnergieQuery(date, resolution)
FrankEnergieClient->>GraphQLAPI: _query(FrankEnergieQuery, headers x-country=country)
GraphQLAPI-->>FrankEnergieClient: response
FrankEnergieClient->>MarketPrices: from_country_dict(response, country)
MarketPrices-->>User: MarketPrices instance
Wijzigingen per bestand
Tips en commando’sInteractie met Sourcery
Je ervaring aanpassenGa naar je dashboard om:
Hulp krijgen
Original review guide in EnglishReviewer's GuideAdds France (FR) market support via the existing x-country mechanism, generalizes BE-specific market price parsing and retrieval into country-agnostic helpers, and fixes handling of null tax/markup fields and missing resolution in marketPrices queries. Sequence diagram for country_prices marketPrices query with resolution and FR supportsequenceDiagram
actor User
participant FrankEnergieClient as FrankEnergie
participant GraphQLAPI
participant MarketPrices
User->>FrankEnergieClient: country_prices(country, start_date, end_date, resolution)
FrankEnergieClient->>FrankEnergieClient: FrankEnergieQuery(date, resolution)
FrankEnergieClient->>GraphQLAPI: _query(FrankEnergieQuery, headers x-country=country)
GraphQLAPI-->>FrankEnergieClient: response
FrankEnergieClient->>MarketPrices: from_country_dict(response, country)
MarketPrices-->>User: MarketPrices instance
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Contributor
|
Warning Review limit reached
Next review available in: 38 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughFrance support was added to country types and parsing. Market-price retrieval was generalized across countries with configurable resolution, while Belgium and France helpers delegate to the shared method. Nullable price fields now default to zero. ChangesCountry market-price support
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant Caller
participant FrankEnergie
participant GraphQLAPI
participant MarketPrices
Caller->>FrankEnergie: country_prices(country, dates, resolution)
FrankEnergie->>GraphQLAPI: Query with country header and variables
GraphQLAPI-->>FrankEnergie: Country marketPrices payload
FrankEnergie->>MarketPrices: Parse country payload
MarketPrices-->>Caller: MarketPrices result
Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - ik heb één issue gevonden en wat algemene feedback achtergelaten:
- In
country_prices/be_prices/fr_pricesis hetresolution-argument geannoteerd alsstr | None, maar wordt het altijd doorgegeven als een niet-NonePriceResolution-string; overweeg het type aan te scherpen (bijv. naarPriceResolution) en dit in lijn te brengen met de bestaandePriceResolutionLiteral voor sterkere typecontrole. - Het gebruik van
data.get(... ) or 0.0inPrice.__init__zal elke falsy waarde (bijv.0, lege string) omzetten naar0.0; als het doel specifiek is omNoneaf te handelen, overweeg dan een explicieteif value is None-controle om te voorkomen dat onverwachte datatypen ongemerkt worden gemaskeerd. - De
end_date-parameter incountry_prices,be_pricesenfr_priceswordt geaccepteerd maar niet gebruikt in de queryvariabelen; als deze niet meer nodig is, overweeg dan deze te verwijderen of in de query op te nemen om verwarring bij aanroepende code te voorkomen.
Prompt voor AI-agents
Los alsjeblieft de opmerkingen uit deze code review op:
## Algemene opmerkingen
- In `country_prices`/`be_prices`/`fr_prices` is het `resolution`-argument geannoteerd als `str | None`, maar wordt het altijd doorgegeven als een niet-None `PriceResolution`-string; overweeg het type aan te scherpen (bijv. naar `PriceResolution`) en dit in lijn te brengen met de bestaande `PriceResolution` Literal voor sterkere typecontrole.
- Het gebruik van `data.get(... ) or 0.0` in `Price.__init__` zal elke falsy waarde (bijv. `0`, lege string) omzetten naar `0.0`; als het doel specifiek is om `None` af te handelen, overweeg dan een expliciete `if value is None`-controle om te voorkomen dat onverwachte datatypen ongemerkt worden gemaskeerd.
- De `end_date`-parameter in `country_prices`, `be_prices` en `fr_prices` wordt geaccepteerd maar niet gebruikt in de queryvariabelen; als deze niet meer nodig is, overweeg dan deze te verwijderen of in de query op te nemen om verwarring bij aanroepende code te voorkomen.
## Individuele opmerkingen
### Opmerking 1
<location path="python_frank_energie/frank_energie.py" line_range="2223-2232" />
<code_context>
+ country: str,
+ start_date: date | None = None,
+ end_date: date | None = None,
+ resolution: str | None = "PT15M",
+ ) -> MarketPrices:
+ """Get market prices for a country served via the 'x-country' header (e.g. BE, FR)."""
if start_date is None:
start_date = datetime.now(UTC).date()
if end_date is None:
end_date = start_date + timedelta(days=1)
- headers = {"x-country": "BE"}
+ headers = {"x-country": country}
query = FrankEnergieQuery(
"""
- query MarketPrices ($date: String!) {
- marketPrices(date: $date) {
+ query MarketPrices ($date: String!, $resolution: PriceResolution!) {
+ marketPrices(date: $date, resolution: $resolution) {
electricityPrices {
</code_context>
<issue_to_address>
**issue (bug_risk):** Het toestaan dat `resolution` `None` is, conflicteert met de niet-null `PriceResolution!` GraphQL-variabele.
Omdat `$resolution` is gedeclareerd als `PriceResolution!`, zal het versturen van `None` een GraphQL-validatiefout veroorzaken. Deze parameter zou niet-optioneel moeten zijn (bijv. `PriceResolution` of `str` zonder `None`), en alle standaardwaardes of normalisatie zouden moeten plaatsvinden vóór het bouwen van de variablenedict.
</issue_to_address>Sourcery is gratis voor open source - als je onze reviews waardeert, overweeg dan om ze te delen ✨
Original comment in English
Hey - I've found 1 issue, and left some high level feedback:
- In
country_prices/be_prices/fr_prices, theresolutionargument is annotated asstr | Nonebut always passed as a non-NonePriceResolutionstring; consider tightening the type (e.g., toPriceResolution) and aligning with the existingPriceResolutionLiteral for stronger typing. - The use of
data.get(... ) or 0.0inPrice.__init__will coerce any falsy value (e.g.,0, empty string) to0.0; if the intent is specifically to handleNone, consider an explicitif value is Nonecheck to avoid inadvertently masking unexpected data types. - The
end_dateparameter incountry_prices,be_prices, andfr_pricesis accepted but not used in the query variables; if no longer needed, consider removing it or wiring it into the query to avoid confusion for callers.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `country_prices`/`be_prices`/`fr_prices`, the `resolution` argument is annotated as `str | None` but always passed as a non-None `PriceResolution` string; consider tightening the type (e.g., to `PriceResolution`) and aligning with the existing `PriceResolution` Literal for stronger typing.
- The use of `data.get(... ) or 0.0` in `Price.__init__` will coerce any falsy value (e.g., `0`, empty string) to `0.0`; if the intent is specifically to handle `None`, consider an explicit `if value is None` check to avoid inadvertently masking unexpected data types.
- The `end_date` parameter in `country_prices`, `be_prices`, and `fr_prices` is accepted but not used in the query variables; if no longer needed, consider removing it or wiring it into the query to avoid confusion for callers.
## Individual Comments
### Comment 1
<location path="python_frank_energie/frank_energie.py" line_range="2223-2232" />
<code_context>
+ country: str,
+ start_date: date | None = None,
+ end_date: date | None = None,
+ resolution: str | None = "PT15M",
+ ) -> MarketPrices:
+ """Get market prices for a country served via the 'x-country' header (e.g. BE, FR)."""
if start_date is None:
start_date = datetime.now(UTC).date()
if end_date is None:
end_date = start_date + timedelta(days=1)
- headers = {"x-country": "BE"}
+ headers = {"x-country": country}
query = FrankEnergieQuery(
"""
- query MarketPrices ($date: String!) {
- marketPrices(date: $date) {
+ query MarketPrices ($date: String!, $resolution: PriceResolution!) {
+ marketPrices(date: $date, resolution: $resolution) {
electricityPrices {
</code_context>
<issue_to_address>
**issue (bug_risk):** Allowing `resolution` to be `None` conflicts with the non-null `PriceResolution!` GraphQL variable.
Because `$resolution` is declared as `PriceResolution!`, sending `None` will cause a GraphQL validation error. This parameter should be non-optional (e.g. `PriceResolution` or `str` without `None`), and any defaulting or normalization should happen before building the variables dict.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
tests/test_models.py (1)
581-586: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse approximate comparisons for floating-point values.
These assertions validate calculated floats; use
pytest.approx(...)to avoid brittle equality checks if the calculation or numeric representation changes.Suggested adjustment
- assert price.market_price == 0.15423 - assert price.market_price_tax == 0.0 - assert price.sourcing_markup_price == 0.0 - assert price.energy_tax_price == 0.0 - assert price.market_price_including_tax == 0.15423 - assert price.market_price_including_tax_and_markup == 0.15423 + assert price.market_price == pytest.approx(0.15423) + assert price.market_price_tax == pytest.approx(0.0) + assert price.sourcing_markup_price == pytest.approx(0.0) + assert price.energy_tax_price == pytest.approx(0.0) + assert price.market_price_including_tax == pytest.approx(0.15423) + assert price.market_price_including_tax_and_markup == pytest.approx(0.15423)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/test_models.py` around lines 581 - 586, Update the floating-point assertions in the price model test to compare calculated values with pytest.approx(...), including market_price, market_price_including_tax, and market_price_including_tax_and_markup, while preserving the expected numeric values and exact zero checks where appropriate.Source: Linters/SAST tools
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@python_frank_energie/frank_energie.py`:
- Around line 2221-2224: Update the market-prices request flow and prices() so a
None resolution is normalized to the existing PT15M default before constructing
GraphQL variables, satisfying the non-null $resolution requirement. Also address
the unused end_date parameter in the surrounding API by wiring it into the
request or removing it consistently.
In `@tests/test_frank_energie.py`:
- Around line 1350-1367: Update test_fr_prices_with_default_dates to compute the
expected date with date.today(), matching the fallback used by fr_prices().
Import date alongside the existing datetime import if needed, and keep the
variables["date"] assertion unchanged.
---
Nitpick comments:
In `@tests/test_models.py`:
- Around line 581-586: Update the floating-point assertions in the price model
test to compare calculated values with pytest.approx(...), including
market_price, market_price_including_tax, and
market_price_including_tax_and_markup, while preserving the expected numeric
values and exact zero checks where appropriate.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: e4cdfe6e-92eb-44a7-ac34-f2fb98bb4b42
📒 Files selected for processing (6)
python_frank_energie/domain.pypython_frank_energie/frank_energie.pypython_frank_energie/models.pypython_frank_energie/types.pytests/test_frank_energie.pytests/test_models.py
- Tighten resolution parameter on country_prices()/be_prices()/ fr_prices()/prices() from `str | None` to the non-optional `PriceResolution` Literal (default "PT15M"). Sending None would violate the GraphQL schema's non-null `$resolution: PriceResolution!` and fail server-side instead of catching the mistake locally. - Replace `data.get(...) or 0.0` with explicit `is None` checks in Price.__init__ so a legitimate falsy API value (e.g. an integer 0) can't be silently confused with a missing field. - Use pytest.approx() for float comparisons in the new null-field regression test (SonarCloud python:S1244).
SonarCloud flagged end_date (python:S1172) as an unused function parameter. Confirmed it was genuinely dead, not just unwired: Frank's marketPrices GraphQL query only accepts a single $date argument. Live-tested adding an $endDate variable and got back "Graphql validation error" -- the API has no such field to receive it. country_prices()/be_prices()/fr_prices() still accept end_date for signature consistency with their existing be_prices()-derived callers; only prices() (the one flagged) is changed here.
|
HiDiHo01
approved these changes
Jul 22, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
FRtoCountryCode(alongsideNL/BE); France is served through the samex-countryheader mechanism already used for Belgium.be_prices()into a sharedcountry_prices(country, ...)helper (be_prices()/newfr_prices()are now thin wrappers), andMarketPrices.from_be_dict()intofrom_country_dict(data, country).Price.__init__crashing (TypeError: unsupported operand type(s) for +: 'float' and 'NoneType') whenmarketPriceTax/sourcingMarkupPrice/energyTaxPricearenull— which France's publicmarketPricesresponse always does (BE always returns numeric values here). They now default to0.0.x-countrymarketPricesquery never requesting a resolution — it silently always returned hourly data regardless of the configured resolution. This affected BE too, just never surfaced.country_prices()/be_prices()/fr_prices()now accept and forward aresolutionparameter (defaultPT15M).Test plan
pytest tests/— 264 passedruff format/ruff check— cleanx-country: FR) and against a running Home Assistant dev container using this library — price fetch succeeds and resolution is honored (96 × 15-min entries/day)Summary by Sourcery
Voeg Frankrijk (FR) toe als ondersteunde x-country markt en generaliseer het ophalen en parsen van land-gescope prijzen.
Nieuwe features:
fr_priceshelper toe die marktprijzen voor Frankrijk ophaalt via de x-country header.country_pricesAPI toe voor het ophalen van marktprijzen voor markten die via x-country worden bediend.Bugfixes:
Priceom crashes op Franse payloads te voorkomen.marketPrices-queries eenresolution-parameter bevatten en respecteren in plaats van altijd uurlijkse data terug te geven.MarketPricesvoor ontbrekende data ofmarketPrices-velden.Verbeteringen:
MarketPrices.from_be_dictnaar een generiekefrom_country_dictdie wordt gebruikt door BE en FR.country_priceshelper te gebruiken, terwijl de bestaandebe_pricesAPI behouden blijft.Tests:
Pricevalideren bij null belasting-/opslagvelden.country_pricesen FR/BE helpers standaard naar en de gevraagderesolutiondoorgeven.Original summary in English
Summary by Sourcery
Add France (FR) as a supported x-country market and generalize country-scoped price fetching and parsing.
New Features:
Bug Fixes:
Enhancements:
Tests:
Summary by CodeRabbit
New Features
Bug Fixes