Add data source LU1 (Luxembourg STATEC)#283
Conversation
Registers the Luxembourg national statistical office (STATEC,
dissemination platform LUSTAT) as data source LU1. LU1 is the
service's own SDMX agency id, so a bare Client("LU1").dataflow()
resolves without an explicit agency argument.
- sources.json: LU1 entry; the supports block disables the five
structural endpoints the service answers with HTTP 404.
- test_sources.py: TestLU1 - endpoint_args for the data endpoint and
an xfail mapping for the endpoints returning HTTP 400
(organisationscheme) and 501 (structure), plus the sdmx-internal
metadata and registration entries.
- test_source.py: bump the expected source count to 36.
- sources.rst: LU1 source section.
- whatsnew.rst: changelog entry.
DataSourceTest against the live service: 13 passed, 21 xfailed.
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Adds a new SDMX data source (LU1) for Luxembourg STATEC (LUSTAT), wiring it into the source registry, docs, and the existing test suite.
Changes:
- Add
LU1entry tosources.jsonwith endpoint URL and declared support flags. - Add a
TestLU1datasource test with endpoint args and expected failures (xfail). - Document the new source and mention it in the “Next release” notes; update source-count assertion.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| sdmx/tests/test_sources.py | Adds a new DataSourceTest subclass for the LU1 source with endpoint config and expected failures. |
| sdmx/tests/test_source.py | Updates the expected count of registered sources to include LU1. |
| sdmx/sources.json | Registers the LU1 source (name, base URL, and supports flags). |
| doc/whatsnew.rst | Adds a “Next release” section entry mentioning the new LU1 source. |
| doc/sources.rst | Documents LU1 with an anchor so it can be referenced from release notes. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| "dataconsumerscheme": false, | ||
| "metadataflow": false, | ||
| "metadatastructure": false, |
There was a problem hiding this comment.
Thanks. This is deliberate, not an oversight. In sdmx1, the supports list and the test's xfail list look similar but mean different things, so endpoints aren't interchangeable between them.
supports is reserved for endpoints the server flatly doesn't have; sdmx1 then skips the request entirely. The endpoints flagged here fail in other ways. Two of them return different error responses, so sdmx1 should still try them and surface a normal, catchable error. The other two, metadata and registration, aren't a limitation of the Luxembourg service at all; they're simply not implemented in sdmx1 yet. All four belong in the test's xfail list, which is where they are.
Detail: per the source-policy in doc/sources.rst, supports: false is for HTTP 404 only. organisationscheme returns HTTP 400 (raising HTTPError) and structure returns HTTP 501 (raising NotImplementedError), so both go in xfail. metadata and registration are sdmx1-internal gaps, xfailed in every source's test (TestTEST, TestAR1, TestUY110). LU1's xfail set is identical to TestUY110's.
| "metadata": NotImplementedError, # Internal to sdmx1 | ||
| "organisationscheme": HTTPError, # 400 Bad Request | ||
| "registration": ValueError, # Internal to sdmx1 | ||
| "structure": NotImplementedError, # 501 Not Implemented |
There was a problem hiding this comment.
This is correct as written. sdmx1 deliberately turns an HTTP 501 ("not implemented") response into Python's built-in NotImplementedError, rather than a generic HTTP error, so that calling code can catch it cleanly. The test expects exactly what the library raises, and the # 501 comment describes the server response that triggers it.
Detail: doc/sources.rst's source-policy states this explicitly. A 501 response raises NotImplementedError; a 400 raises HTTPError. Verified live: probing LU1's structure endpoint raised NotImplementedError, and the DataSourceTest xfails cleanly (a wrong exception type would make it error rather than xfail). TestUY110 carries the identical structure: NotImplementedError # 501.
| ``LU1``: STATEC (Luxembourg) | ||
| ---------------------------- |
There was a problem hiding this comment.
Fair point: the name does appear in slightly different forms across the registry, the test, and the docs. We've kept each as it is because they each follow the pattern the repo already uses. The documentation heading in particular has a fixed shape, so it can't simply mirror the others.
Detail: every source heading in doc/sources.rst uses the same form, namely the source ID followed by the office name and the country in parentheses. For example, NB is "Norges Bank (Norway)" and LSD is "National Institute of Statistics (Lithuania)". NB itself varies the same way across its registry name ("Norges Bank (NO)"), its test docstring ("Norges Bank.") and its heading. Happy to standardise on a specific form if you'd prefer one.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #283 +/- ##
==========================================
- Coverage 99.04% 98.27% -0.77%
==========================================
Files 116 116
Lines 9503 9507 +4
==========================================
- Hits 9412 9343 -69
- Misses 91 164 +73
🚀 New features to boost your workflow:
|
|
Thanks for identifying this new source, and I'm glad to hear the package is useful for you and that you're interested to make further contributions! FYI, while I have not yet made a specific decision to prohibit contributions created using ‘AI’ tools, I will very likely do so in the near future, probably by adopting a code of conduct that makes clear why their use is not acceptable. Please keep in mind that the best way to support volunteer maintainers of useful open-source packages (such as me) is to remember and respect that we have limited time, and show that respect by thinking about how your actions (including, but not limited to, your choice to use ‘AI’ tools) affect others. In this case:
I have not read through (2) and (3) in detail, as I sense that would be a waste of my time. I sense from a quick skim that your replies (3) are rejecting the Copilot outputs (2), so there is no actual effect on the code; the changes in the original commits are the ones that I should actually review. Puzzling over (1) also consumed some of my time. If my finite time was wasted looking at the Copilot output or your prompts to it, I would be left with less or no time to actually interact with the human contributor, i.e. you. Likewise, when you spend time writing ‘to’ Copilot, you are sacrificing time you could spend communicating with me. For a contrasting example of good practice, see #225 opened by (I believe) a colleague of yours. Notice that there are several long comments from both him and I, in which we think about the information that we need to share with the other person and how best to do that. Unlike reading AI output, this kind of communication is valuable and a productive use of time. In any case, I will try to review and merge this PR. However, please:
|
Thank you very much for your time, and I sincerely apologize for the confusion. When I opened PR #282, I didn't realize GitHub would automatically trigger a Copilot review. I was confused by the automated output, which is why I closed the original PR and tried opening a new one. When Copilot popped up again on the new PR, I honestly had no idea how to turn it off. I figured it must be a required check for the sdmx1 repo, so I just did my best to reply to its comments. I have since reconfigured my GitHub settings, so Copilot will not generate automated reviews on my pull requests moving forward. I apologize again for the extra noise. I haven't been active on GitHub for a while and wasn't caught up on these new AI integration features. I completely agree with your stance on AI-generated content and deeply respect your time as a maintainer. To avoid any further issues, I will make sure to open an issue to provide context and discuss any bugs or features with you before submitting a new PR. Thank you also for letting me know about my colleague’s contributions to the repository; I will be sure to coordinate with him on related issues in the future. Thank you again for your patience and for your work on this package. |
OK, thanks for explaining. I'm aware that Microsoft (GitHub) have been very aggressive about automatically enabling both (a) permissions on repos to receive/accept Copilot reviews and (b) individual contributors' accounts to generate/request such reviews. They do not make it obvious or easy to turn these off. I will also see if there is anything I can do in the sdmx1 settings to prevent them. |
Adds the Luxembourg national statistical office (STATEC, dissemination platform LUSTAT) as data source
LU1.LU1is the service's own SDMX agency id — every dataflow and structure is maintained under it — so a bareClient("LU1").dataflow()works without an explicitagency_id.I'm Dinh Nguyen-Xuan, from the Statistics Department of the International Monetary Fund. I really appreciate the
sdmx1package and am exploring ways to help enhance it.PR checklist