Broken labels in aperture.plot()

[laurenmarietta]

(@shanosborne asked me to open an issue for this.) I have been using pysiaf's plotting ability in one of my notebooks for a while now, like so:

nc_siaf = pysiaf.Siaf('NIRCam')

plt.figure(figsize=(15,10))
for apername in sorted(nc_siaf.apernames):
    a = apername
    if ('_FULL' in a) and ('OSS' not in a) and ('MASK' not in a) and (a[-1] != 'P'):
        nc_siaf[a].plot(frame='tel', name_label=True, fill_color='white')
plt.gca().invert_xaxis()

However, with the latest updates of pysiaf (not sure if it was 0.2.4 or 0.2.5 or something else), the name_label=True parameter doesn't seem to be working as expected anymore. Instead of turning on the name labels for each aperture in the plot, now all the name labels are displaying as "True".

When I tried label=True instead of name_label=True, I got a legend over the plot that listed every aperture on the plot being labeled as "True".

In this specific example I can get the behavior I want (the aperture names appearing as colored text in the plot) by setting name_label=a, but it seems to me like the labelling shouldn't work this way.

[shanosborne]

Thanks Lauren - I'll start looking into this!

[shanosborne]

So there's 2 plotting functions in pysiaf, one for plotting apertures one at a time (the plot() method in aperture.py) and one for plotting multiple apertures at once (the plot() method that in siaf.py that inherits the aperture.py plot() method). In your example, since you plot using nc_siaf[a], a single aperture, you go through the aperture.py plot() method which has the name_label attribute where you pass it a string of the name of your aperture (or anything you’d like). So a fix to your code would be setting name_label=a:

for apername in sorted(nc_siaf.apernames):
    a = apername
    if ('_FULL' in a) and ('OSS' not in a) and ('MASK' not in a) and (a[-1] != 'P'):
        nc_siaf[a].plot(frame='tel', name_label=a, fill_color='white')

Or there’s also the option to pass name_label=‘default’ which will also plot the aperture names (for some reason this wasn’t included in the docstring, but I’ve updated that).

The label keyword doesn’t exist in this aperture.py plot() method, it only exists in the siaf.py plot() method. That keyword takes a boolean and should pick out the correct names to plot. Now, it doesn’t currently do that, but that’s its intention and I’ve also fixed that (it will be in a new PR)

So (once the label keyword problem is fixed) if you wanted instead to plot multiple apertures at once (and call the siaf.py plot() method), you could provide a list of apertures (passed into names) and then set label=True.

aperlist = [a for a in sorted(nc_siaf.apernames) if 
            (('_FULL' in a) and ('OSS' not in a) and ('MASK' not in a) and (a[-1] != 'P'))]
nc_siaf.plot(names=aperlist, frame='tel', label=True, fill_color='white')

[shanosborne]

@Johannes-Sahlmann:
I do think it’s a bit unintuitive to have 2 similarly named keyword arguments (name_label and label) that accept different data types (string vs boolean). And it’s exacerbated by the fact that if someone thinks name_label and label work the same way, passing name_label a boolean doesn’t raise an error anywhere, it instead plots that boolean for all the labels. I think it would be smart to fix this, but that could be done several ways. My initial suggestion is changing name_label to label in the aperture.py plot() and let it take either a boolean or a string. Do you agree that this is a source of confusion for users and/or have a different idea about how to rectify it?

[laurenmarietta]

Ooh, I didn't know there was another plot() function in siaf.py. That definitely looks like the one I should be using in this case.

Thanks for the clarification, and for thinking about the weirdness of label versus name_label. (I, for the record, agree that it is unintuitive.)

[Johannes-Sahlmann]

@shanosborne I agree with your assessment concerning name_label and label and it would be great if you can fix this.
As background information, this non-ideal setup resulted from merging initially independent plotting methods (the ones from from jwxml and my own).