astropy / specutils

An Astropy coordinated package for astronomical spectroscopy. Maintainers: @rosteen @keflavich @eteq
http://specutils.readthedocs.io/en/latest/
166 stars 127 forks source link

Update __str__ and __repr__ for Spectrum1D #1123

Closed rosteen closed 7 months ago

rosteen commented 7 months ago

Fixes #1073, as well as improving the readability/concision of the __repr__ format (especially for things like large cubes). Still tweaking this but it's close, tests will need updating as well.

codecov[bot] commented 7 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Comparison is base (425eab3) 71.07% compared to head (4019868) 70.93%.

:exclamation: Current head 4019868 differs from pull request most recent head 1e00460. Consider uploading reports for the commit 1e00460 to get more accurate results

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #1123 +/- ## ========================================== - Coverage 71.07% 70.93% -0.14% ========================================== Files 61 61 Lines 4249 4215 -34 ========================================== - Hits 3020 2990 -30 + Misses 1229 1225 -4 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

pllim commented 7 months ago

Looks like your devdeps job pulls in numpy 2.0.dev but I also see warnings, so please inspect the log carefully.

https://github.com/astropy/specutils/actions/runs/7832040522/job/21369775774?pr=1123

rosteen commented 7 months ago

Looks like your devdeps job pulls in numpy 2.0.dev but I also see warnings, so please inspect the log carefully.

https://github.com/astropy/specutils/actions/runs/7832040522/job/21369775774?pr=1123

As far as I could tell the only warning was from an upstream package, nothing to do with this. This PR actually fixes some numpy-related errors, although I think that's due to a pre-2.0 update. I got a thumbs-up for this offline so I'll probably merge after tests pass rather than wait for more input (unless you want to give feedback on the actual changes, Pey Lian).

pllim commented 7 months ago

I think the actual diff looks reasonable. I think the new str and repr are nicer. Thanks!

rosteen commented 7 months ago

Test failure was a timeout, merging.