I think this is ready to go now.

If anyone has a moment to check this out, it would be most appreciated.

If we get this merged I think it will allow actions to pass on some of the other pending PRs so we can get those finished up.

@FoamyGuy thanks for making this code more modular between the two types with the new LabelBase class. As for the code, this may be a stylistic question, but I'm curious to understand how you chose to extract the arguments from the **kwargs with the kwargs.get commands.

To me, it's easier to understand which variables the __init__ function accepts is by including it explicitly in the subclass def __init__(argument1, argument2...). For the arguments that you want to handle, but still pass along to super, you can be clear about that when you call the super().__init__(font, **kwargs) like you do for the font argument.

For example, the scale parameter is actually ignored by the superclass but it's not obvious here. If you handle scale in the subclass but don't pass it up to super then maybe that will be obvious when others review the code.

This isn't anything that's a show stopper, but maybe will make it easier to clarify which variables are used in which classes/subclasses.

As for sphinx docs, I think putting the doc strings in both is just fine for now. Main question is whether they get out of sync in the future with changes. But keeping the doc strings in the subclass (that most people will use) will probably make life easier so folks don't have to traipse through the superclasses. I explored some of dealing with inheritance with Sphinx but I haven't yet been able to get it as clean as I wanted, so I think your choice on Docs here is a good one.

I did a few random checks of the setters (both label and bitmap_label) and all worked ok. I think this looks good, thanks for making this code a lot cleaner with the refactor.

@kmatch98 Thanks for having a look! My main motivation with the __init__ arguments not being listed in the base class was to try to avoid the duplicate code check failure.

In the original actions failure from the issue it flagged this chunk of code for the duplication:

adafruit_display_text/label.py:1:0: R0801: Similar lines in 2 files
==adafruit_display_text.bitmap_label:85
==adafruit_display_text.label:76
        color=0xFFFFFF,
        background_color=None,
        line_spacing=1.25,
        background_tight=False,
        padding_top=0,
        padding_bottom=0,
        padding_left=0,
        padding_right=0,
        anchor_point=None,
        anchored_position=None, (duplicate-code)

I do think the code would be overall more clear if they were listed in the base class instead of using the kwargs "lookup". But I'm not sure of a way to include them while minding the duplicate check.

Seems lame for pylint to complain about but so it goes...
Looks good to me.

I agree, in this case I wish it was just a touch smarter about finding duplicates. It does seem to ignore some things since the docstrings currently exist in both and don't get flagged, I wish it would allow arguments similarly.

A few other options possibly worth consideration:

1. Change the order of the arguments and include them in both places. I'm not sure if this is "cheating" really. It does seem to be against the spirit of the no duplication rule. But it would get rid of actual duplication that would get flagged I think.

2. Further logically split the arguments into ones used specifically used in the base class and subclasses. As-is many of the arguments are "not used" in the base class __init__ function and the pylint check for it is disabled. Those unused ones could be moved out of Base and included in the subclasses which seems more correct to me from an "OOP" perspective. But many of them would get used in both subclasses so we'd be back to this pylint duplicate code issue.

These were the only other ideas I had, but I think the drawbacks outweigh both of them personally.

I tested this with some code, and it seems ok, if any glitches we could fix them after. Thanks @FoamyGuy for all this work,

Thank you @kmatch98 and @jposada202020 for looking into this and testing the changes. We can see if there is any further feedback tomorrow. I'll come back tomorrow night to merge this if there is nothing further.

Once this is merged it will unlock a few of the other PRs.

@FoamyGuy Thank you for working on this. That would be great :). I did more testing as I was also testing the new "directional" label with this Refactor library. works good