[SyneRBI/SIRF] SIRF #1357 (PR #1378)
Dimitra-Kyriakopoulou
Changes in this pull request
-- Fixed #1357 by applying the same direction already discussed upstream, not a new design.
This patch follows the __array_priority__ / reverse-operator approach that later appears in #1373. #1358 is
already merged.
-- Changes:
- add
__array_priority__ = 10toArrayContainer - add
__radd__,__rsub__, and__rtruediv__toDataContainer - add regression coverage for the reported left-hand cases in
src/common/Utilities.py
-- Checked the reported cases: ay * x, 1 + x, ax + x, ax - x, ay / x, 2 / x
These now return SIRF containers instead of numpy.ndarray or TypeError.
Testing performed
- reran the explicit
#1357repro on the patched branch - ran
src/xSTIR/pSTIR/tests/tests_data_container_algebra.py -v
Related issues
Checklist before requesting a review
- I have performed a self-review of my code
- I have added docstrings/doxygen in line with the guidance in the developer guide
- I have implemented unit tests that cover any new or modified functionality
- The code builds and runs on my machine
- CHANGES.md has been updated with any functionality change
Contribution Notes
Please read and adhere to the contribution guidelines.
Please tick the following:
- [x ] The content of this Pull Request (the Contribution) is intentionally submitted for inclusion in SIRF (the Work) under the terms and conditions of the Apache-2.0 License.
You can view, comment on, or merge this pull request online at:
https://github.com/SyneRBI/SIRF/pull/1378
Commit Summary
- b94de3b Fix mixed NumPy/SIRF left-hand algebra (#1357)
- 6015037 Temporarily enable CI on Issue_1357 branch
- e8c95a7 Restore build-test workflow triggers
File Changes
(2 files)
- M src/common/SIRF.py (10)
- M src/common/Utilities.py (40)
Patch Links:
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
Kris Thielemans
KrisThielemans left a comment (SyneRBI/SIRF#1378)Thanks @Dimitra-Kyriakopoulou. can you clarify? This is lifting the _array_priority_ to top-level in the SIRF hierarchy?
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
Dimitra-Kyriakopoulou
Dimitra-Kyriakopoulou left a comment (SyneRBI/SIRF#1378)THANK YOU WHOLEHEARTEDLY FOR YOUR MESSAGE!!
Yes, the intent was to lift __array_priority__ to ArrayContainer, i.e. to the common Python base for SIRF array containers, instead of relying on per-subclass behavior. That makes NumPy mixed expressions prefer SIRF container arithmetic more consistently. So it is a lift within the Python array-container branch (not to the top of the whole SIRF hierarchy).
Also i attach the pdf which describes my 1st attempt to project 6: i uploaded in
________________________________
Από: Kris Thielemans ***@***.***>
Στάλθηκε: Τετάρτη, 11 Μαρτίου 2026 12:40 μμ
Προς: SyneRBI/SIRF ***@***.***>
Κοιν.: Dimitra-Kyriakopoulou ***@***.***>; Mention ***@***.***>
Θέμα: Re: [SyneRBI/SIRF] SIRF #1357 (PR #1378)
[https://avatars.githubusercontent.com/u/6362141?s=20&v=4]KrisThielemans left a comment (SyneRBI/SIRF#1378)<https://github.com/SyneRBI/SIRF/pull/1378#issuecomment-4038242947>
Thanks @Dimitra-Kyriakopoulou<https://github.com/Dimitra-Kyriakopoulou>. can you clarify? This is lifting the _array_priority_ to top-level in the SIRF hierarchy?
—
Reply to this email directly, view it on GitHub<https://github.com/SyneRBI/SIRF/pull/1378#issuecomment-4038242947>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/ACZD3XRJC6M6OA5FKWJK4MT4QE7BZAVCNFSM6AAAAACWN7X7B6VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHM2DAMZYGI2DEOJUG4>.
You are receiving this because you were mentioned.Message ID: ***@***.***>
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
Dimitra-Kyriakopoulou
Dimitra-Kyriakopoulou left a comment (SyneRBI/SIRF#1378)THANK YOU WHOLEHEARTEDLY FOR YOUR MESSAGE!!
Yes, the intent was to lift __array_priority__ to ArrayContainer, i.e. to the common Python base for SIRF array containers, instead of relying on per-subclass behavior. That makes NumPy mixed expressions prefer SIRF container arithmetic more consistently. So it is a lift within the Python array-container branch (not to the top of the whole SIRF hierarchy).
Also i attach the pdf which describes my 1st attempt to project 6: i uploaded in
THANK YOU WHOLEHEARTEDLY FOR EVERYTHING!!!
Dimitra
Από: Kris Thielemans ***@***.***>
Στάλθηκε: Τετάρτη, 11 Μαρτίου 2026 12:40 μμ
Προς: SyneRBI/SIRF ***@***.***>
Κοιν.: Dimitra-Kyriakopoulou ***@***.***>; Mention ***@***.***>
Θέμα: Re: [SyneRBI/SIRF] SIRF #1357 (PR #1378)
[https://avatars.githubusercontent.com/u/6362141?s=20&v=4]KrisThielemans left a comment (SyneRBI/SIRF#1378)<https://github.com/SyneRBI/SIRF/pull/1378#issuecomment-4038242947>
Thanks @Dimitra-Kyriakopoulou<https://github.com/Dimitra-Kyriakopoulou>. can you clarify? This is lifting the _array_priority_ to top-level in the SIRF hierarchy?
—
Reply to this email directly, view it on GitHub<https://github.com/SyneRBI/SIRF/pull/1378#issuecomment-4038242947>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/ACZD3XRJC6M6OA5FKWJK4MT4QE7BZAVCNFSM6AAAAACWN7X7B6VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHM2DAMZYGI2DEOJUG4>.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
Kris Thielemans
KrisThielemans left a comment (SyneRBI/SIRF#1378)ok. @evgueni-ovtchinnikov will review this. thanks!
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
Evgueni Ovtchinnikov
evgueni-ovtchinnikov left a comment (SyneRBI/SIRF#1378)so far as I can see, this PR just duplicates #1373 - I suggest we rather merge #1373, and if my understanding is correct just drop this one
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
Dimitra-Kyriakopoulou
Dimitra-Kyriakopoulou left a comment (SyneRBI/SIRF#1378)Dear Professor @evgueni-ovtchinnikov and Professor @KrisThielemans,
I just checked this and Professor Ovtchinnikov is absolutely right. I had myself explicitly referenced #1373, because I understood it as the upstream direction for fixing #1357 rather than as an already complete implementation of it. Because it was still open, and under the time pressure here, I did not realise that #1373 already contained the same functional changes, and in fact, as I now see, also more follow-up work (extra result-type checks and docs/changelog updates). I am deeply sorry for taking your time on this, and I wholeheartedly thank Professor Ovtchinnikov for pointing it out and saving me from unintentionally duplicating existing work -in fact in a way that could have come close to indirect plagiarism.
I AM DEEPLY SORRY,
Dimitra
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
Kris Thielemans
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
Kris Thielemans
KrisThielemans left a comment (SyneRBI/SIRF#1378)no problem. Closing this.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.