# Rename meas_deblender's python/lsst/meas/deblender/deblend.py

XMLWordPrintable

#### Details

• Type: RFC
• Status: Implemented
• Resolution: Done
• Component/s:
• Labels:
None

#### Description

meas_deblender contains the file python/lsst/meas/deblender/deblend.py, which defines SourceDeblendTask and SourceDeblendConfig.

meas_deblender also contains the file python/lsst/meas/deblender/baseline.py, which contains — amongst other things — the function deblend.

Finally, it contains the file python/lsst/meas/deblender/__init__.py, which does:

 from .baseline import * ... from .deblend import * 

The intention here seems to be to lift the contents of baseline.py into the lsst.meas.deblender namespace. Unfortunately, the deblend() function is shadowed by deblend.py, and so isn't accessible:

 [ins] In [1]: import lsst.meas.deblender    [ins] In [2]: lsst.meas.deblender.deblend  Out[2]:    [ins] In [3]: lsst.meas.deblender.baseline.deblend  `Out[3]:

This, in itself, is a fairly minor inconvenience (anybody trying to call lsst.meas.deblender.deblend() has presumably long since given up). However, it means that documentation for deblend() is not accessible to Sphinx using the standard .. automodapi:: lsst.meas.deblender syntax.

There are a couple of ways to make these docs appear. First, one could add a .. automodapi:: lsst.meas.deblender.baseline. That works, but has the downside that the non-shadowed content of baseline.py now appears in the documentation twice. One could fix that by not lifting baseline into lsst.meas.deblender... but that's an incompatible API change.

My proposed alternative is to rename deblend.py to sourceDeblendTask.py. This is in-keeping with the spirit (if not quite the letter) of the coding standards.

Currently, SourceDeblendTask is accessible as both lsst.meas.deblender.SourceDeblendTask and lsst.meas.deblender.deblend.SourceDeblendTask. This change would remove the latter, thereby introducing a backwards-incompatible change to the API. I contend that all reasonable consumers are using the former (a contention which is supported by a brief grep of our existing code), so I expect breakage to be minimal. I therefore suggest that making this change without the normal deprecation period would be appropriate. However, I'd welcome feedback from the community in general and the DM-CCB in particular as to whether they agree.

#### Activity

Hide
Tim Jenness added a comment -

You could go a bit further and rename it to _sourceDeblendTask.py to make it even clearer that people aren't supposed to address is directly.

Show
Tim Jenness added a comment - You could go a bit further and rename it to _sourceDeblendTask.py to make it even clearer that people aren't supposed to address is directly.
Hide
John Swinbank added a comment -

... rename it to _sourceDeblendTask.py ...

I don't feel strongly about this, but I note that that convention is not used elsewhere in this package so it seems a bit inconsistent.

Show
John Swinbank added a comment - ... rename it to _sourceDeblendTask.py ... I don't feel strongly about this, but I note that that convention is not used elsewhere in this package so it seems a bit inconsistent.
Hide
John Parejko added a comment -

+1

I support this change: the correlation between Task class names and the files they live in is not very strong, and making it stronger is good, regardless of the particular docs/API fix listed here (which is also good).

Show
John Parejko added a comment - +1 I support this change: the correlation between Task class names and the files they live in is not very strong, and making it stronger is good, regardless of the particular docs/API fix listed here (which is also good).
Hide
Kian-Tat Lim added a comment -

OK with me, and without deprecation period as anyone doing it the wrong (deep) way deserves what they get

Show
Kian-Tat Lim added a comment - OK with me, and without deprecation period as anyone doing it the wrong (deep) way deserves what they get
Hide
Fred Moolekamp added a comment -

+1
I've never really been happy with this modules naming structure. The only thing I would add is that when I created meas_extensions_scarlet I unfortunately made a similar oversight, creating a deblend function inside of deblend.py. So my recommendation would be to also rename deblend.py in meas_extensions_scarlet to scarletDeblendTask.py.

Show
Fred Moolekamp added a comment - +1 I've never really been happy with this modules naming structure. The only thing I would add is that when I created meas_extensions_scarlet I unfortunately made a similar oversight, creating a deblend function inside of deblend.py . So my recommendation would be to also rename deblend.py in meas_extensions_scarlet to scarletDeblendTask.py .
Hide
John Swinbank added a comment - - edited

Following today's DM-CCB meeting, I understand the “board recommended” status to mean that the DM-CCB are happy for this change to be made with no deprecation period. On that basis, I'll file a triggering ticket to change both meas_deblender and meas_extensions_scarlet (thanks Fred Moolekamp) and adopt the RFC on its due date, barring any objections.

Show
John Swinbank added a comment - - edited Following today's DM-CCB meeting, I understand the “board recommended” status to mean that the DM-CCB are happy for this change to be made with no deprecation period. On that basis, I'll file a triggering ticket to change both meas_deblender and meas_extensions_scarlet (thanks Fred Moolekamp ) and adopt the RFC on its due date, barring any objections.
Hide
John Swinbank added a comment -

Looking at meas_extensions_scarlet more closely, I notice that it is not currently possible to use lsst.meas.extensions.scarlet.ScarletDeblendTask; rather, the only name that exists is lsst.meas.extensions.scarlet.deblend.ScarletDeblendTask. This is documented and used outside the project, so I don't think we can just change it.

I propose to adopt this RFC, with the implementation being to do the rename in meas_deblender, and to add a deprecation notice to meas_extensions_scarlet so that it can be fixed at some future date.

Show
John Swinbank added a comment - Looking at meas_extensions_scarlet more closely, I notice that it is not currently possible to use lsst.meas.extensions.scarlet.ScarletDeblendTask ; rather, the only name that exists is lsst.meas.extensions.scarlet.deblend.ScarletDeblendTask . This is documented and used outside the project , so I don't think we can just change it. I propose to adopt this RFC, with the implementation being to do the rename in meas_deblender, and to add a deprecation notice to meas_extensions_scarlet so that it can be fixed at some future date.

#### People

Assignee:
John Swinbank
Reporter:
John Swinbank
Watchers:
Fred Moolekamp, John Parejko, John Swinbank, Kian-Tat Lim, Tim Jenness
0 Vote for this issue
Watchers:
5 Start watching this issue

#### Dates

Created:
Updated:
Resolved:
Planned End:

#### Jenkins

No builds found.