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-37532: Fix meas_base imports #28

Merged
merged 1 commit into from Feb 11, 2023
Merged

DM-37532: Fix meas_base imports #28

merged 1 commit into from Feb 11, 2023

Conversation

mwittgen
Copy link
Contributor

No description provided.

@@ -35,7 +35,7 @@
import lsst.afw.table as afwTable
import lsst.geom
import lsst.meas.base as measBase
from lsst.meas.base.fluxUtilities import FluxResultKey
from lsst.meas.base import FluxResultKey
Copy link
Member

Choose a reason for hiding this comment

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

If we are going to import FluxResultKey from measBase, we might as well not do it and use it FluxResultKey with the namespace specified wherever it is used.

Copy link
Member

Choose a reason for hiding this comment

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

Yes, the code is already importing measBase so may as well use measBase.FluxResultKey in here and remove the second import completely.

Copy link
Contributor Author

@mwittgen mwittgen Jan 30, 2023

Choose a reason for hiding this comment

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

Implemented to use measBase and tested locally against meas_base changes in
lsst/meas_base#233
as well with the latest daily release.

Copy link
Contributor

@erykoff erykoff left a comment

Choose a reason for hiding this comment

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

Looks good!

@arunkannawadi
Copy link
Member

One minor nitpicky comment is that the commit messages should begin with capitalized letter, according to the Tim Pope's best committing practices linked from our dev guide: https://developer.lsst.io/work/flow.html#git-commit-message-best-practices

@mwittgen mwittgen changed the title DM-37532: fix meas_base imports DM-37532: Fix meas_base imports Feb 10, 2023
@mwittgen
Copy link
Contributor Author

Capitalized git commit message. Will fix messages in the related pybind packages as well.

@mwittgen mwittgen merged commit 687cf57 into main Feb 11, 2023
@mwittgen mwittgen deleted the tickets/DM-37532 branch February 11, 2023 01:06
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

4 participants