Ruff #141

taldcroft · 2025-01-10T14:17:19Z

Description

Scratching the ruff itch again, now with xija.

This includes some non-trivial fixes along the way after the --unsafe-fixes commit. I tried to isolate those changes. This will need functional testing with xija_gui_fit.

Interface impacts

None.

Testing

Unit tests

Mac

(ska3-flight-2025.0rc2) ➜  xija git:(ruff) git rev-parse --short HEAD          
5ae0456
(ska3-flight-2025.0rc2) ➜  xija git:(ruff) pytest
============================================================ test session starts =============================================================
platform darwin -- Python 3.12.8, pytest-8.3.4, pluggy-1.5.0
rootdir: /Volumes/git
configfile: pytest.ini
plugins: doctestplus-1.3.0, anyio-4.7.0, timeout-2.3.1
collected 44 items                                                                                                                           

xija/tests/test_get_model_spec.py .......                                                                                              [ 15%]
xija/tests/test_models.py .....................................                                                                        [100%]

============================================================ 44 passed in 37.44s =============================================================

Independent check of unit tests by Jean

osx

(ska3) flame:xija jean$ pytest xija
================================================ test session starts =================================================
platform darwin -- Python 3.12.8, pytest-8.3.4, pluggy-1.5.0
rootdir: /Users/jean/git
configfile: pytest.ini
plugins: anyio-4.7.0, timeout-2.3.1
collected 44 items                                                                                                   

xija/tests/test_get_model_spec.py .......                                                                      [ 15%]
xija/tests/test_models.py .....................................                                                [100%]

================================================== warnings summary ==================================================
xija/xija/tests/test_models.py::test_dpa_real[True]
  /Users/jean/miniforge3/envs/ska3/lib/python3.12/site-packages/setuptools_scm/git.py:312: UserWarning: git archive did not support describe output
    warnings.warn("git archive did not support describe output")

-- Docs: https://docs.pytest.org/en/stable/how-to/capture-warnings.html
=========================================== 44 passed, 1 warning in 33.96s ===========================================
(ska3) flame:xija jean$ git rev-parse HEAD
ce3f16a4a862dab2082107a31e5e3f71760da819

Functional tests

From this repo:

python -m xija.gui_fit.app acisfp_spec_matlab --stop 2025:001

Then:

Freeze all pars
Thaw a couple of power pars
Fit
Histogram
Save model
Show radzones, limits
Annotate line
Model info
Move sliders.

All OK.

jeanconn · 2025-02-07T20:02:15Z

xija/gui_fit/app.py

@@ -282,7 +282,7 @@ def __init__(self, model, main_window):

    def toggle_all_data(self, state):
        checked = state == QtCore.Qt.Checked
-        for i, box in enumerate(self.check_boxes):
+        for _i, box in enumerate(self.check_boxes):


Could probably just cut the enumerate ...

Yes, but then I would need to redo all the testing.

jeanconn · 2025-02-07T20:08:45Z

xija/gui_fit/fitter.py

        ui.add_user_pars("xijamod", self.model.parnames)
        ui.set_model(1, "xijamod")
        calc_stat = CalcStat(self.model, self.child_pipe, self.maxiter)
        ui.load_user_stat("xijastat", calc_stat, lambda x: np.ones_like(x))
+        xijastat = globals()["xijastat"]


would ui.set_state("xijastat") make as much sense if we're sticking with the ui versions?

I see this was changed again in a later commit, though I think my question is still valid.

would ui.set_state("xijastat") make as much sense if we're sticking with the ui versions?

I don't understand the question. Make as much as sense as what, and what is the context about sticking with ui versions?

I think these are mostly OBE, in that messing with this code was more trouble than its worth and probably out of scope for ruff (despite my earlier effort).

Regarding "I don't understand the question. Make as much as sense as what, " Sorry I was hoping that was clear in context and I had a typo. I meant - would ui.set_stat("xijastat") make as much sense as what you did in the first commit - where what you did was to define xijastat and then supply it with ui.set_state(xijastat) (no quotes). Because ui.set_stat will take either the string or the stat.

With regard to sticking with the ui versions, I thought that the problem with the "ui" versions of the sherpa functions is that they do insert into the namespace, but for most of the tasks one can instead use the OO sherpa pieces directly.

Because ui.set_stat will take either the string or the stat.

I'm guessing you trusted the documentation instead of trying it. I've been down that road...

File "/Volumes/git/xija/xija/gui_fit/fitter.py", line 146, in fit ui.set_stat("xijastat") # type: ignore # noqa: F821, PGH003 ^^^^^^^^^^^^^^^^^^^^^^^ File "<string>", line 1, in set_stat File "/Users/aldcroft/miniconda3-arm/envs/ska3-flight-2025.0rc2/lib/python3.12/site-packages/sherpa/ui/utils.py", line 3076, in set_stat statobj = self._get_stat_by_name(stat) ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ File "/Users/aldcroft/miniconda3-arm/envs/ska3-flight-2025.0rc2/lib/python3.12/site-packages/sherpa/ui/utils.py", line 2899, in _get_stat_by_name raise ArgumentErr('badstat', name) sherpa.utils.err.ArgumentErr: 'xijastat' is not a valid statistic

Ah. Does set_state(str) only work for the built-in statistics? I did not see that advertised. Thanks!

jeanconn · 2025-02-07T20:08:57Z

xija/gui_fit/fitter.py

@@ -139,10 +139,12 @@ def fit(self):
        ui.set_method(self.method)
        ui.get_method().config.update(sherpa_configs.get(self.method, {}))
        ui.load_user_model(CalcModel(self.model), "xijamod")  # sets global xijamod
+        xijamod = globals()["xijamod"]


Where is this one used?

Resolved I think in a later commit though still a little confusing with ui .

I really don't understand how these sherpa functions inject into the namespace. It is really unfortunate that ui.load_user_model doesn't return the model, but there you go.

Base automatically changed from improved-model-spec-file to master January 28, 2025 18:22

taldcroft force-pushed the ruff branch from 6937f76 to ce3f16a Compare January 29, 2025 12:10

taldcroft added 11 commits January 30, 2025 13:23

Update for linting and formatting with ruff

ede1d3b

Format with ruff (no quote changes)

60a9829

Format with ruff (only quote changes)

7d52a16

ruff check --fix and reformat

e2da78e

ruff check --fix --unsafe-fixes

1ffe8c7

Remove try import ska_matplotlib (it is always available)

d427707

Remove try import cheta (it is always available)

c19b420

Get local sherpa object references from globals()

b457405

Fix setting dP values

86973a5

Final hand edits

690c152

Put back logging imports to clogging module

5ae0456

taldcroft force-pushed the ruff branch from ce3f16a to 5ae0456 Compare January 30, 2025 18:26

taldcroft marked this pull request as ready for review January 30, 2025 22:28

taldcroft requested review from jzuhone and jeanconn January 30, 2025 22:28

Revert setting xijamod and xijastat via globals() - does not work

87ad521

jzuhone approved these changes Feb 3, 2025

View reviewed changes

jeanconn reviewed Feb 7, 2025

View reviewed changes

jeanconn approved these changes Feb 10, 2025

View reviewed changes

taldcroft merged commit 744ed7b into master Feb 10, 2025
2 checks passed

taldcroft deleted the ruff branch February 10, 2025 12:10

This was referenced Mar 18, 2025

Update sot/xija to 4.33.2 sot/skare3#1508

Open

2025.4 sot/skare3#1514

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Ruff #141

Ruff #141

taldcroft commented Jan 10, 2025 •

edited by jeanconn

Loading

jeanconn Feb 7, 2025

taldcroft Feb 8, 2025

jeanconn Feb 7, 2025

jeanconn Feb 7, 2025

taldcroft Feb 8, 2025

jeanconn Feb 8, 2025 •

edited

Loading

taldcroft Feb 8, 2025

jeanconn Feb 8, 2025

jeanconn Feb 7, 2025

jeanconn Feb 7, 2025

taldcroft Feb 8, 2025

Ruff #141

Ruff #141

Conversation

taldcroft commented Jan 10, 2025 • edited by jeanconn Loading

Description

Interface impacts

Testing

Unit tests

Functional tests

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jeanconn Feb 8, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

taldcroft commented Jan 10, 2025 •

edited by jeanconn

Loading

jeanconn Feb 8, 2025 •

edited

Loading