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-37918: Change MetricMeasurementBundle import path #832

Merged
merged 1 commit into from Apr 27, 2023

Conversation

natelust
Copy link
Contributor

Checklist

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

@natelust natelust requested a review from timj April 26, 2023 23:21
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.

This change is fine if you remove the underscore part of the name.

@@ -391,6 +391,6 @@ storageClasses:
ScarletModelData:
pytype: lsst.meas.extensions.scarlet.ScarletModelData
MetricMeasurementBundle:
pytype: lsst.analysis.tools.analysisMetrics.MetricMeasurementBundle
pytype: lsst.analysis.tools.interfaces._metricMeasurementBundle.MetricMeasurementBundle
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
pytype: lsst.analysis.tools.interfaces._metricMeasurementBundle.MetricMeasurementBundle
pytype: lsst.analysis.tools.interfaces.MetricMeasurementBundle

we don't include underscores in the name -- get_full_type_name elides them from the hierarchy (that's deliberate because the underscore is meant to be an internal implementation details)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I must not be understanding something in how this works then:

In [1]: import lsst.analysis.tools.interfaces.MetricMeasurementBundle
---------------------------------------------------------------------------
ModuleNotFoundError                       Traceback (most recent call last)
Cell In[1], line 1
----> 1 import lsst.analysis.tools.interfaces.MetricMeasurementBundle

ModuleNotFoundError: No module named 'lsst.analysis.tools.interfaces.MetricMeasurementBundle'

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This was resolved on slack by Tim pointing out I was being dense.

@codecov
Copy link

codecov bot commented Apr 26, 2023

Codecov Report

Patch coverage: 92.30% and project coverage change: +0.09 🎉

Comparison is base (2e0fd7a) 87.68% compared to head (0905b11) 87.77%.

❗ Current head 0905b11 differs from pull request most recent head 457685e. Consider uploading reports for the commit 457685e to get more accurate results

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #832      +/-   ##
==========================================
+ Coverage   87.68%   87.77%   +0.09%     
==========================================
  Files         268      268              
  Lines       35066    35032      -34     
  Branches     7383     7375       -8     
==========================================
+ Hits        30746    30748       +2     
+ Misses       3162     3125      -37     
- Partials     1158     1159       +1     
Impacted Files Coverage Δ
python/lsst/daf/butler/formatters/parquet.py 93.67% <71.42%> (ø)
tests/test_parquet.py 99.16% <100.00%> (ø)

... and 8 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@natelust natelust force-pushed the tickets/DM-37918 branch 2 times, most recently from fb652b7 to 0905b11 Compare April 27, 2023 01:52
@natelust natelust merged commit 4ebb90a into main Apr 27, 2023
8 of 9 checks passed
@natelust natelust deleted the tickets/DM-37918 branch April 27, 2023 02:19
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

2 participants