Fix raster legend assertion bug

[nkorinek]

@lwasser Here's a PR fixing the issue brought up in https://github.com/earthlab/matplotcheck/issues/163 . While my last attempt lead to me thinking this issue was unsolvable, I reviewed my old code and realized I was going about it in the entirely wrong way! This solution is a lot more elegant and efficient, and it seems to work! If I need to add in any tests to go alongside this please let me know. Thanks!

[codecov[bot]]

Codecov Report

Merging #226 into master will not change coverage. The diff coverage is n/a.

@@           Coverage Diff           @@
##           master     #226   +/-   ##
=======================================
  Coverage   79.93%   79.93%           
=======================================
  Files          19       19           
  Lines        1829     1829           
=======================================
  Hits         1462     1462           
  Misses        367      367           

---

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update df8995a...df8995a. Read the comment docs.

[lwasser]

@nkorinek i just slacked you - if you can please clean up the gist and clearly show me how this works / doesn't work. I think we need to capture some errors here and return more useful information to the user. so let's use that gist as a way to clean this up just a bit so i can see how it works and suggest how i think it should work in terms of user input.

[nkorinek]

@lwasser updated gist here! https://gist.github.com/nkorinek/e8aa5ec22cdaa8cca4acd2789f6031ff if you run it locally the error thrown will be different since I updated them to make more since.

[lwasser]

@nkorinek i see a LOT of merge master commits in this pr. i suspect we need to find a way for you to update from master at the CLI or rebase... let's try a few things for your next few pr's

[nkorinek]

This should be ready for a review btw @lwasser . The docs are only failing b/c of that m2r bug.

[lwasser]

ok @nkorinek sorry to do this to you. this has a conflict and needs an update from master to fix m2r. i'll have a look again when ci is passing. you may want to check in on some of your other pr's as well as they likely all need updates.

[lwasser]

/rebase

[nkorinek]

@lwasser This is (finally) updated! Took a bit of time to figure out how the old tests were working, and how I wanted to change them. This is basically a complete overhaul of this function. I'd be happy to talk about the changes I made to the functions, I think it'd be good to bounce this idea off of someone else before it gets merged, but I think it's a big improvement over how this used to run! Let me know if you have any immediate questions.

[lwasser]

so this code

        # matches that entry
        legend_dict = {}
        for p in [
            p
            for sublist in [leg.get_patches() for leg in legends]
            for p in sublist
        ]:
            label = p.get_label().lower()
            legend_dict[p.get_facecolor()] = _which_label(
                label, all_label_options
            )

creates a dictionary with color: mapped to the output of which label. i thijnk we definitely want an output dictionary that pulls the students plot legend legend_label: "color found". And what we could do is then have another item in the dictionary representing whether the label is True (matches an expected label) or False (does not match an expected label.

If which_label is renamed to be _check_label and then the docstring explains that this helper does a check to ensure the label in the student plot is found in the expected label values, that would be valuable. Once we have that we can then test that the color of the legend - label matches the value we expect in the image array. i think this will work well.