sherpa / sherpa

Fit models to your data in Python with Sherpa.
https://sherpa.readthedocs.io
GNU General Public License v3.0
155 stars 50 forks source link

Opt cleanup #2047

Open DougBurke opened 4 months ago

DougBurke commented 4 months ago

Summary

Improve the typing coverage of the sherpa.optmethods code and internal code clean up.

Details

This is a follow-on to #2045 and takes in some of the changes in #2015. However, there is less of an attempt to mark symbols as deprecated as these are definitely internal routines for this module. Much of the code removal is to remove debugging information that only someone who could edit the code could get to see. This, along with some typing annotation, and some understanding of the code from #2007 / #2022, has lead to some minor code cleanups. I am also conscious of improving the documentation given https://github.com/sherpa/sherpa/discussions/2042

It has now been updated to build on #2079 and I haven't yet updated this text

The typing rules that are added are meant to try and represent the intent of the interfaces, rather than exactly matching the reality. For example, there's subtle differences between a Sequence and ndarray that can cause mypy to complain over things that are "obviously" correct - e.g. can you call len on the object - as well as subtle differences between float and SupportsFloat that do not really change the code. I claim that until we have all the major parts working together it's hard to nail down these types, but that it has actually shown be some problems (or potential problems): for example the `fix logical error with population_size argument" commit.

I have added some notes of possible future work (none of them would be significant changes), but it's hard to make these changes as those code paths are currently not tested.

The idea is that this PR should not cause difference in behavior with code using Sherpa unless they are doing something really funky.

codecov[bot] commented 4 months ago

Codecov Report

Attention: Patch coverage is 98.98990% with 1 line in your changes missing coverage. Please review.

Project coverage is 87.46%. Comparing base (a02a7bf) to head (04e9b3c).

Files with missing lines Patch % Lines
sherpa/optmethods/opt.py 95.83% 1 Missing :warning:
Additional details and impacted files [![Impacted file tree graph](https://app.codecov.io/gh/sherpa/sherpa/pull/2047/graphs/tree.svg?width=650&height=150&src=pr&token=9aWhIDJHGx&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=sherpa)](https://app.codecov.io/gh/sherpa/sherpa/pull/2047?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=sherpa) ```diff @@ Coverage Diff @@ ## main #2047 +/- ## ========================================== + Coverage 87.42% 87.46% +0.03% ========================================== Files 84 84 Lines 29673 29677 +4 Branches 4463 4461 -2 ========================================== + Hits 25941 25956 +15 + Misses 3620 3609 -11 Partials 112 112 ``` | [Files with missing lines](https://app.codecov.io/gh/sherpa/sherpa/pull/2047?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=sherpa) | Coverage Δ | | |---|---|---| | [sherpa/optmethods/\_\_init\_\_.py](https://app.codecov.io/gh/sherpa/sherpa/pull/2047?src=pr&el=tree&filepath=sherpa%2Foptmethods%2F__init__.py&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=sherpa#diff-c2hlcnBhL29wdG1ldGhvZHMvX19pbml0X18ucHk=) | `95.58% <100.00%> (-0.13%)` | :arrow_down: | | [sherpa/optmethods/ncoresde.py](https://app.codecov.io/gh/sherpa/sherpa/pull/2047?src=pr&el=tree&filepath=sherpa%2Foptmethods%2Fncoresde.py&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=sherpa#diff-c2hlcnBhL29wdG1ldGhvZHMvbmNvcmVzZGUucHk=) | `42.03% <100.00%> (ø)` | | | [sherpa/optmethods/ncoresnm.py](https://app.codecov.io/gh/sherpa/sherpa/pull/2047?src=pr&el=tree&filepath=sherpa%2Foptmethods%2Fncoresnm.py&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=sherpa#diff-c2hlcnBhL29wdG1ldGhvZHMvbmNvcmVzbm0ucHk=) | `33.00% <100.00%> (ø)` | | | [sherpa/optmethods/optfcts.py](https://app.codecov.io/gh/sherpa/sherpa/pull/2047?src=pr&el=tree&filepath=sherpa%2Foptmethods%2Foptfcts.py&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=sherpa#diff-c2hlcnBhL29wdG1ldGhvZHMvb3B0ZmN0cy5weQ==) | `77.44% <100.00%> (+0.67%)` | :arrow_up: | | [sherpa/utils/types.py](https://app.codecov.io/gh/sherpa/sherpa/pull/2047?src=pr&el=tree&filepath=sherpa%2Futils%2Ftypes.py&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=sherpa#diff-c2hlcnBhL3V0aWxzL3R5cGVzLnB5) | `100.00% <100.00%> (ø)` | | | [sherpa/optmethods/opt.py](https://app.codecov.io/gh/sherpa/sherpa/pull/2047?src=pr&el=tree&filepath=sherpa%2Foptmethods%2Fopt.py&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=sherpa#diff-c2hlcnBhL29wdG1ldGhvZHMvb3B0LnB5) | `71.60% <95.83%> (+0.35%)` | :arrow_up: | ... and [1 file with indirect coverage changes](https://app.codecov.io/gh/sherpa/sherpa/pull/2047/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=sherpa) ------ [Continue to review full report in Codecov by Sentry](https://app.codecov.io/gh/sherpa/sherpa/pull/2047?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=sherpa). > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=sherpa) > `Δ = absolute (impact)`, `ø = not affected`, `? = missing data` > Powered by [Codecov](https://app.codecov.io/gh/sherpa/sherpa/pull/2047?dropdown=coverage&src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=sherpa). Last update [a02a7bf...04e9b3c](https://app.codecov.io/gh/sherpa/sherpa/pull/2047?dropdown=coverage&src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=sherpa). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=sherpa).
DougBurke commented 2 months ago

Rebased as I try to make sherpa.utils.types a thing.

DougBurke commented 3 weeks ago

Rebased to pick up recent changes and re-ordered the commits.