Skip to content

Ruff #141

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 12 commits into from
Feb 10, 2025
2 changes: 2 additions & 0 deletions xija/gui_fit/fitter.py
Original file line number Diff line number Diff line change
Expand Up @@ -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"]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Where is this one used?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

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"]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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).

Copy link
Contributor

@jeanconn jeanconn Feb 8, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

ui.set_stat(xijastat)

# Set frozen, min, and max attributes for each xijamod parameter
Expand Down