Feature/sweep post process#336
Conversation
nulinspiratie
left a comment
There was a problem hiding this comment.
I think this functionality better belongs in results folder because it is similar to fetching tool, which is located there.
Additionally, there is the XarrayDataFetcher in Qualibration-Libs. I think the behavior of this tool should mimic that one, i.e. be used as a context manager that can also retrieve data while still acquiring. In fact, we should consider merging the functionality of this feature and the XarrayDataFetcher into one tool.
It's fine if the current version doesn't support data fetching while acquiring, but I think we should at least match the UX.
Since XarrayDataFetcher is in QualibrationLibs, we should think together with @OziEgri where the final tool should reside, py-qua-tools or qualibration-libs? I think it makes sense to have one repository for this and related tools
The PR is also missing a README, I'm sending you a draft separately @natangqm
|
I also propose we ditch the naming "sweep program", it could confuse customers. I propose to instead move the contents to the file |
nulinspiratie
left a comment
There was a problem hiding this comment.
Approved, but minor docs comments to implement before merging
| wait_until_done: If True, block until the job completes before fetching results. | ||
| Defaults to False. |
There was a problem hiding this comment.
It is unclear here what wait_until_done=False actually does. Earlier in the docstring it already says that it retrieves result streams from a completed QUA job, so what happens if it's not complete? Does it acquire whatever is available, or something else?
This could be clarified in the docstring, and also the README.
There was a problem hiding this comment.
Its clarified in the readme and in the docstring
| return stream_data | ||
|
|
||
|
|
||
| def fetch_xarray_data( |
There was a problem hiding this comment.
One thing to consider here is that from a user's perspective they would likely want to keep acquiring until the job is done, then do a final acquisition, and then use that as the final dataset, so something like:
while not job.is_done(): # forgot the exact command here
ds = fetch_xarray_data(job, iterables)
ds = fetch_xarray_data(job, iterables) # Acquire one last timeThis is a bit clunky, so we should come up with a better solution. For example, in the previous xarray data fetcher, it was done through a for loop, something like:
for ds in fetch_data(job, iterables): # again, forgot the exact command
...
# No need for a final fetch, the `for` loop logic already encapsulates thisI'm not saying this is the right way, but we should have something simple that avoids us having to implement the logic in the first example. We don't need to do it here, but we need a solution in the next phase
There was a problem hiding this comment.
we can do that in next version
…m-qua Version-gate the QUA-iterable post-processing API (added in qm-qua 1.3.1) via MIN_QM_QUA_VERSION as the single source of truth, raising a clear ImportError on older qm-qua while keeping qualang_tools importable. Feature tests skip below the minimum; add packaging as an explicit dependency. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
@OziEgri Any objections we merge this? |
pytestmark runs after module import, so it can't prevent the collection-time ImportError from the test modules' top-level qm-qua 1.3.1 imports (e.g. declare_with_stream) on qm-qua 1.2.6 (the Python 3.9 leg). Use collect_ignore in conftest, gated on qua_iterables_supported(), to avoid importing those modules entirely when qm-qua is too old. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The 1.3.0a1 pre-release lacks STREAM_NAME_SEPARATOR; the API requires 1.3.1. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
NativeIterable/NativeIterableRange/NativeIterableBase are deprecated since qm-qua 1.3.0 (removed in 2.0.0). Use PythonIterable/PythonIterableRange/ PythonIterableBase across the fetch_xarray_data tests and README examples. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
@OziEgri please review |
OziEgri
left a comment
There was a problem hiding this comment.
LGTM, would be good to add Theo or Kevin to the review, as they have more experience here
Summary
Adds
fetch_xarray_data— fetches results from a completed QUA job and returns them as anxarray.Dataset, with each result stream automatically shaped and labelled according to the QUA iterables (sweep) structure. Native iterables are reassembled from their per-index streams, and axes are transposed so the Dataset's dimension ordering matches the original iteration order.qm-qua compatibility
The feature relies on qm-qua APIs introduced in 1.3.1 (e.g.
STREAM_NAME_SEPARATOR,declare_with_stream, thequa_iteratorsextensions). Rather than raise the package-wide floor and break users on older qm-qua, the requirement is handled gracefully:TYPE_CHECKING+from __future__ import annotations), soimport qualang_toolsworks on any supported qm-qua.fetch_xarray_datais version-gated: on qm-qua < 1.3.1 it raises a clearImportError("fetch_xarray_data requires qm-qua>=1.3.1 ...")instead of an opaque import crash.MIN_QM_QUA_VERSION = "1.3.1"is the single source of truth, driving the runtime gate, error message, and test skips.The
qm-quafloor inpyproject.tomlstays at>=1.2.3. Because qm-qua 1.3.1 dropped Python 3.9, the lock resolves per-interpreter: 1.3.1 on Python 3.10+, 1.2.6 on Python 3.9 — 3.9 support is preserved.Changes
qualang_tools.results.fetch_xarray_data(+ README inqua_iterables_processing/).xarraypromoted from optional extra to a core dependency;packagingadded as an explicit core dependency (used for the version comparison).collect_ignore(their top-level qm-qua 1.3.1 imports can't be collected on older qm-qua, so apytest.mark.skipifis insufficient).CI
Green across the matrix: Python 3.10/3.12/3.13 install qm-qua 1.3.1 and run the feature tests; Python 3.9 installs 1.2.6 and cleanly skips them.
🤖 Generated with Claude Code