Interesting proposal. I think this is probably the correct solution, but wonder if folks use the _ sentinel to skip some entries in a list of handles?

Interesting proposal. I think this is probably the correct solution, but wonder if folks use the _ sentinel to skip some entries in a list of handles?

Perhaps, but they would be using undocumented behaviour. In this case, this change would make those entries appear in the legend, yes. However, they are already passing a list explicitly (something the documentation treats separately from passing no arguments). It should be fairly straightforward to filter the list manually. On the other hand, the API currently doesn't allow for labels with leading underscores at all, even though it should be possible according to the documentation. This is how we arrived at this PR after all.

So it seems an example is failing due to the change in logging method. Reviewing the contribution guidelines, it is not entirely clear to me which warning should be emitted with the logging module and which should use _api.warn_external. If I understand correctly, I should not have replaced logging with _api.warn_external, because we want (I am assuming) a warning for every invocation of legend on a particular line in the source when no labels are found, not only one (implying that something is wrong with that one line of code, where in reality it also depends on how plot was called - with or without labels). Can I get confirmation on this?

I very strongly prefer that we update the documentation to match the behavior rather than changing the behavior. In general, when the code and the docs conflict we prefer correcting the documentation

If we do want to change this behavior, then I think that this should be treated as an API change (not a bug fix) and would need to go through the deprecation cycle.

It looks like this code ended up in Legend via 2b6927d which unified Axes.legend and Figure.legend. From there the history moves across files, but it looks like the behavior of uniformly dropping any entries starting with _ has been true for a decade. I think the analysis that needs to be done is to actually sort out what version we started filtering explicitly passed labels.

One a more fundamental level, hiding labels with a leading underscore at all is a questionable design decision. If the only purpose of a label is to display an element in the legend, we could perfectly live with empty (or None) labels defining "no legend entry". I suspect that underscore-labels are abused as an id, why else would somebody want to set them? We could move away from this concept, but that would also be a longer deprecation effort. Not sure if it's worth it.

The distinction between logging and warnings is https://docs.python.org/3/howto/logging.html#logging-basic-tutorial

warnings.warn() in library code if the issue is avoidable and the client application should be modified to eliminate the warning

logging.warning() if there is nothing the client application can do about the situation, but the event should still be noted.

The fundamental idea is that any good code should be warnings-free, so use warnings if you want the user to modify their code.
I'm 80% sure that calling legend with no data to display is a usage error, which would warrant using warnings, but there may be some special ways of modifying content after creation. To keep things simple, I recommend to stick with logging for now.

I very strongly prefer that we update the documentation to match the behavior rather than changing the behavior. In general, when the code and the docs conflict we prefer correcting the documentation

To me, the documentation make sense. When automatic element detection is used, additional processing is performed to extract labels following some set of rules. However, when passing the elements explicitly, this step is skipped, and so the user can pass any label they wish, even one with a leading underscore. Updating the documentation to match the behaviour seems like we are removing functionality (that is in fact documented, but not implemented). I would very much like to be able to use labels with leading underscores, but as it stands this is not possible at all. Even if I were to try and use the Legend class manually - since this is where the piece of code in question resides.

Perhaps a middle ground would be to move this code to matplotlib.legend._parse_legend_args (where it should be anyway), without altering the API behaviour. This way one would not need to create a new class that inherits from Legend and overrides its constructor to use such labels. Instead, one could instantiate Legend and give it to Axes as in Axes.legend.

One a more fundamental level, hiding labels with a leading underscore at all is a questionable design decision. If the only purpose of a label is to display an element in the legend, we could perfectly live with empty (or None) labels defining "no legend entry". I suspect that underscore-labels are abused as an id, why else would somebody want to set them? We could move away from this concept, but that would also be a longer deprecation effort. Not sure if it's worth it.

Here is my example use case for leading underscores. I am making a plotting utility that uses a dictionary of simulation parameters to determine parameter labels, units, symbols, names, etc. It uses some default labels with leading underscores to handle missing parameter attributes. This way the user can still see the label, and it is also obvious that it is a default-generated label, as it is using the leading underscore convention that has special meaning in python.

The analysis that needs to be done is to sort out exactly when we started to filter the explicit labels, I think that will influence if we think going through the deprecation dance is worth it or not.

I think the proposed refactor is OK.

Suggestion: add a kwarg, "exclude_with_underscore=True" (default for current behavior) or "False" for the @Klayto-235 use case. Lot's of possibilities for the kwarg name, e.g., "drop_leading_underscore", etc.

Apologies for missing the discussion on the call, but I would think that if the user explicitly passes a list of artists then we should indeed use them as is without dropping underscored labels; I would also not bother with adding an option as [a for a in artists if not a.get_label().startswith("_")] seems easy enough to type and more explicit (you don't need to remember an option name which shows up nowhere else, etc.).

Note that a standard deprecation path is available:

• 3.x (when the deprecation starts): warn if the user passes a list where any artist has an underscored label
• 3.x+2: switch the behavior and remove the warning; users who handled the warning above by filtering themselves will not see a behavior change

For reference, I suspect(?) the current behavior came in via #2442 (mpl 1.4.0), though bisecting to such an old version of matplotlib would be somewhat tricky.

Sorry for not being there as well. I follow @anntzer's arguments.

It looks like this one needs devs' consensus on the way forward. FWIW, @anntzer's solution makes sense to me.

@Klayto-235 are you still interested in working on this if we find that consensus?

I'm fine with @anntzer's recommendation.