Skip to content
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

DM-33013: Add ipython display function to correctly print object #621

Merged
merged 1 commit into from Dec 22, 2021

Conversation

mfisherlevine
Copy link
Contributor

Checklist

  • ran Jenkins
  • added a release note for user-visible changes to doc/changes

@codecov
Copy link

codecov bot commented Dec 17, 2021

Codecov Report

Merging #621 (da29a6e) into main (4f83673) will increase coverage by 0.01%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #621      +/-   ##
==========================================
+ Coverage   84.12%   84.14%   +0.01%     
==========================================
  Files         237      237              
  Lines       30300    30305       +5     
  Branches     5008     5008              
==========================================
+ Hits        25491    25499       +8     
+ Misses       3664     3661       -3     
  Partials     1145     1145              
Impacted Files Coverage Δ
python/lsst/daf/butler/core/dimensions/_records.py 84.76% <100.00%> (+2.21%) ⬆️
tests/test_simpleButler.py 97.84% <100.00%> (+0.02%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4f83673...da29a6e. Read the comment docs.

@mfisherlevine mfisherlevine force-pushed the tickets/DM-33013 branch 2 times, most recently from 7f1ab38 to faa1b54 Compare December 21, 2021 22:48
@mfisherlevine mfisherlevine marked this pull request as ready for review December 22, 2021 00:03
Copy link
Member

@timj timj left a comment

Choose a reason for hiding this comment

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

I'm not a huge fan of special-purpose display methods but this does need a docstring and it also needs a test to make sure it doesn't crash when it runs.

The test can capture stdout or something (with _repr_html_ it's easier because that's a return value).

python/lsst/daf/butler/core/dimensions/_records.py Outdated Show resolved Hide resolved
@mfisherlevine mfisherlevine force-pushed the tickets/DM-33013 branch 5 times, most recently from 3eb3ccc to dbafd6a Compare December 22, 2021 01:17
Copy link
Member

@TallJimbo TallJimbo left a comment

Choose a reason for hiding this comment

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

Minor comment on the test code. Otherwise looks good to me.

tests/test_simpleButler.py Outdated Show resolved Hide resolved
tests/test_simpleButler.py Outdated Show resolved Hide resolved
python/lsst/daf/butler/core/dimensions/_records.py Outdated Show resolved Hide resolved
@mfisherlevine mfisherlevine merged commit 9fe239b into main Dec 22, 2021
@mfisherlevine mfisherlevine deleted the tickets/DM-33013 branch December 22, 2021 19:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants