search

SuperDARN / pydarn

Python library for visualizing SuperDARN Data
GNU Lesser General Public License v3.0
31 stars 11 forks source link

Fan plots drop the rightmost beam #139

Closed SEC-VT closed 3 years ago

SEC-VT commented 3 years ago

BUG

In plotting/fan.py, the _plotfan method does not plot the last beam in a scan.

Priority

Example of the bug

This is an example of the output from the plotting/fan.py/Fan._plotfov method, overlaid with a scatter plot of the thetas and rs output. Note the extra beam clockwise of the FOV. image The same beam is dropped in the _plotfan routine which actually plots the vectors.

Attempts

I'm working on a PR that adds cartopy functionality to the fan plots which addresses this issue.

Data Location

Using INV and BKS fitacfs from Virginia Tech.

Potential Bug Location

The issue stems from lines 299-314 (in _plotfov):

        if boundary:
            # left boundary line
            plt.polar(thetas[0:ranges[1], 0], rs[0:ranges[1], 0],
                      color='black', linewidth=0.5)
            # top radar arc
            plt.polar(thetas[ranges[1] - 1, 0:thetas.shape[1] - 1],
                      rs[ranges[1] - 1, 0:thetas.shape[1] - 1],
                      color='black', linewidth=0.5)
            # right boundary line
            plt.polar(thetas[0:ranges[1], thetas.shape[1] - 2],
                      rs[0:ranges[1], thetas.shape[1] - 2],
                      color='black', linewidth=0.5)
            # bottom arc
            plt.polar(thetas[0, 0:thetas.shape[1] - 2],
                      rs[0, 0:thetas.shape[1] - 2], color='black',
                      linewidth=0.5)

The right boundary line and bottom arc are subtracting 2 from the shape, instead of 1. The top radar arc should not subtract anything. Similarly, in line 183 (_plotfan method):

        # Begin plotting by iterating over ranges and beams
        for gates in range(ranges[0], ranges[1]-1):
            for beams in range(thetas.shape[1] - 2):
                # Index colour table correctly
                cmapindex = (scan[gates, beams] + abs(zmin)) /\

"thetas.shape[1] - 2" should only subtract 1.

Potential Solution(s)

See above. Subtracting 1 instead of 2 should fix it.

Extra Notes

Please provide other pertinent details about this feature:

billetd commented 3 years ago

Hi @SEC-VT, thank you for pointing this out.

When the code was first written, I was sure there was a reason why it was "-2" and not "-1", but for the life of my I can't figure out why now. I just tested the fix and seems like no problems arose, and it wasn't pulling random data from somewhere it shouldn't be.

I'll make a new PR now so this can immediately be fixed.

mts299 commented 3 years ago

@SEC-VT Thank you again for helping with this, make sure to work from the develop branch.

Also, I have a branch already made for just installing cartopy ish to pyDARN if you want to look at it. Also, can you make an issue New Featur about the cartopy work you are doing so others don't work on it at the same time.

Thanks again!!

SEC-VT commented 3 years ago

@billetd Awesome! I hesitated raising this issue for a bit because it seemed intentional and I thought I was missing something. 😄

@mts299 Will do! RE: scope of PRs in general, is there a preference for one big one or many smaller PRs (all related to fan plots)? There are few improvements to the fan plot routine I've got in the oven (cartopy, datetime input, performance optimizations, etc.) and I don't want to unintentionally bombard the group with notifications.

carleyjmartin commented 3 years ago

I wouldn't worry about notifications, we can turn off what we don't want to see! Little ones will get through review quicker too.

And if you think there is a problem, don't worry about raising an issue, the worst thing that will happen is you get an answer as to why and the issue gets closed, but then if anyone else has that question then it's already documented in an issue to read.

Also, there is already an issue for the datetime input for fan plots that I brought up #124 , so if you're working on it feel free to assign yourself to that as I haven't worked on it in this repo, but I did write the code already for our website to get a record number from datetime obj.