Fixed plot bugs, added functionality, updated images #353

ventsiR · 2024-10-28T19:19:11Z

Added functions in awpy.plot.utils that transform in the opposite direction (i.e. from map pixel coordinates to in-game coordinates) and added documentation indicating direction of transform.

Updated the vertigo map images to show the new layout after the map got updated.

Added functions in awpy.plot.utils that transform in the opposite direction (i.e. from map pixel coordinates to in-game coordinates) and added documentation indicating direction of transform. Updated the vertigo map images to show the new layout after the map got updated.

…ke Vertigo and Nuke) awpy.plot.utils.is_position_on_lower_level() was always returning False trivially; if statement in line 122 was checking if pos is greater than max instead off less than max. Added `is_lower` argument to all plot functions. plot() (and _generate_frame_plot() & gif(), which use it) now correctly set alpha to 0.4 for lower points. heatmap() now ignores points not on the level provided by the user and spits out a warning. Also cleaned up map_data file.

vary_alpha functionality of plot.heatmap() was not working correctly because values were set to np.nan before applying alpha arithmetic. I also added vary_alpha_range as an argument for users to set min and max alpha for each point

Added ignore_extreme_points argument. When set to True, it will ignore points that are outside of the bounds of the default map image. Previously, code functioned as if this was set to False. Default value is False.

pnxenopoulos

I really appreciate the PR! There are some interesting and helpful additions here. I think the PR looks mostly good and I left some more clarification/discussion-based comments. Once we sort those minor comments out, we can merge this into the 2.0.0b5 branch. I should just make it 2.0, but alas...

pnxenopoulos · 2024-11-24T23:23:01Z

awpy/plot/plot.py

@@ -40,6 +46,10 @@ def plot(  # noqa: PLR0915
            - 'armor': int (0-100)
            - 'direction': Tuple[float, float] (pitch, yaw in degrees)
            - 'label': str (optional)
+        ignore_extreme_points (bool, optional): If set to True, will ignore


This is an interesting proposition. Is there any reason we would want to allow this in the first place? Or, in other words, do you think there's any danger to making this a default?

If I were designing this function from the ground-up, I would have the default set to True.

In the commit I have set it to default as False because it mimics the previous behavior of the function (i.e. it plotted points way outside of the map). My reasoning was backwards compatibility for people who have already written code with the current behavior in mind.

I'm kind of new to GitHub and commits so I'm leaving the decision up to you :D

Totally understand the backwards compatibility concerns. I think given that this is still a beta version, we can feel ok with breaking the previous functionality, particularly in this case, where there is little noticeable value to plotting points outside the map. I say we remove this arg and make it default behavior! It's a nice safeguard that I overlooked when first writing the function.

Sounds good, will remove the arg and integrate the functionality.

pnxenopoulos · 2024-11-24T23:25:07Z

awpy/plot/plot.py

@@ -22,14 +23,19 @@
 def plot(  # noqa: PLR0915
    map_name: str,
    points: Optional[List[Tuple[float, float, float]]] = None,
+    is_lower: Optional[bool] = False,


What if this parameter were reworked into a "lower fraction"? As in, we assume that upper points are plotted with alpha = 1.0, and then lower are lower_frac*alpha ? This would allow for a more expressive form of what it appears you are going for.

Which, we may also want to offer a param for the "dead player alpha" that we default to 0.15, too

Good idea!

The only thing to note is that when is_lower = True, it also changes the image to the lower part of the map, but then again, you can control this via the map_name argument.

Good point. Perhaps in the docstring we can say something like "if you want to plot a specific lower part of a map, use *_lower. However, then this could imply that a user may want to plot only the top part of the map, too. Perhaps we search for a "_lower" and "_upper" if we want to plot only those.

Now that I look at this, perhaps we want to just have a generic game_to_pixel(tuple). That tuple can be an (x,y) pair. What do you think?

pnxenopoulos · 2024-11-24T23:26:33Z

awpy/data/map_data.py

-            {"name": "default", "altitude_max": 10000, "altitude_min": -5},
-            {"name": "lower", "altitude_max": -5, "altitude_min": -10000},
-        ],
+        "lower_level_max": -5.0,


I like this simplification, but I may offer a rewording -- perhaps lower_level_max_units or something. Just to put the "units" into the name (which unfortunately in this case is just generic "units").

Sounds good!

Sidebar: I am a bit new to GitHub & commits; is it good etiquette for me to implement your feedback, or to let you implement it?

I think it depends from person-to-person, repo-to-repo, but usually the one who opened the PR will make the changes, unless it's a super small change.

Sounds good. Will implement everything you've mentioned; they're pretty small changes and everything is still relatively fresh in my mind :D

pnxenopoulos · 2024-11-24T23:30:40Z

awpy/plot/plot.py

        size (int, optional): Size of the heatmap grid. Defaults to 10.
        cmap (str, optional): Colormap to use. Defaults to 'RdYlGn'.
        alpha (float, optional): Transparency of the heatmap. Defaults to 0.5.
        vary_alpha (bool, optional): Vary the alpha based on the density. Defaults
            to False.
+        vary_alpha_range (List[float, float], optional): The min and max transparency


I'm a bit unclear as to what this parameter is doing -- do you mind explaining again? I realize I don't quite remember what I wrote 🤣

Previously (before I added vary_alpha_range) when vary_alpha was set to True, it would plot the densest points with alpha equal to the alpha argument, the least dense points with alpha = 0, and anything in between would be assigned linearly between 0 and alpha.

vary_alpha_range lets the user set a custom range (instead of 0 and alpha).

I now realize that vary_alpha_range might be a confusing name - it was intended to sound like "the range of argument vary_alpha", but it sounds like, "vary the alpha's range".

Can't think of anything better though - I considered alpha_range, but it's even more ambiguous :D

I see. If an alpha range is specified, should we assume the user wants to vary_alpha? This may allow us to remove one arg. If so, then I think having alpha_range is a fine name, as long you add just a tiny bit more explanation in the docstring!

Great idea!

pnxenopoulos · 2024-11-24T23:31:29Z

awpy/plot/utils.py

@@ -34,10 +34,40 @@ def position_transform_axis(
    return (start - position) / scale


+def position_revert_axis(


What's the intended use for this function?

Given a point that is plotted on an image, users can feed the point's pixel co-ordinates into position_revert_axis() and get the in-game co-ordinates of that point (assumes that user didn't modify the dimensions of the image from the default 1024x1024).

I am using this function in my code to plot points on a matplotlib graph, draw a polygon over it, and return my polygon to a database in terms of in-game coordinates.

I see. Perhaps what we can do is rename the function. Usually for these transformer-style function we can name them like "X_to_Y". We probably should rename the position_transform() function, we can still do that, I'm all ears for names!

I agree; I didn't take the liberty due to - again - backwards compatibility reasons.

Maybe rename the functions to in-game_to_pixel_transform & pixel_to_in-game_transform and create "new" functions that go by the old names (position_transform_axis & position_revert_axis) that simply call in-game_to_pixel_transform & pixel_to_in-game_transform and send a deprecation warning in the console?

I think we can do the following:

game_to_pixel(...)
pixel_to_game(...)

since "in game" is redundant with "game" and "transform" is implied by the "to"

then, the interface is much cleaner and imo, easy to tell what the functions do

pnxenopoulos · 2024-11-25T02:05:39Z

Also, DM me on Discord so I know which account you are!

Renaming of variables/functions, removal/additions of function arguments.

More renaming (should've pushed it with last commit)

pnxenopoulos · 2024-11-26T02:37:54Z

awpy/plot/plot.py

-                )
+            if (
+                transformed_x < 0
+                or transformed_x > 1024


Because these are constants, it would be helpful to have a comment above here stating that these are the maximum expected bounds

pnxenopoulos · 2024-11-26T02:40:17Z

awpy/plot/utils.py

+    This is the old name of function `game_to_pixel_axis`. Please update
+    your code to avoid future deprecation.
+    """
+    warnings.warn(


Great idea to add the deprecation warnings. I think we can drop the _renaming_warning() however, and just include the specific strings in the deprecation messages!

pnxenopoulos · 2024-11-26T02:41:00Z

awpy/plot/utils.py

+    return pixel_to_game_axis(map_name, position, axis)
+
+
+def position_transform(


I also think these functions would still live in plot, rather than live in utils (so the import path would be the same)

pnxenopoulos · 2024-11-26T02:46:24Z

awpy/plot/utils.py

@@ -1,15 +1,16 @@
 """Utilities for plotting and visualization."""

 from typing import Literal
+import warnings


should generally do

import warnings from typing import Literal from awpy.data.map_data import MAP_DATA

Applying linting fixes to the code

ventsiR added 2 commits October 28, 2024 15:14

ventsiR changed the title ~~Added plot.utils functions and updated map images~~ Fixed bug with awpy.plot not handling lower levels points, added plot.utils reverse transform functions and updated map images Oct 29, 2024

ventsiR changed the title ~~Fixed bug with awpy.plot not handling lower levels points, added plot.utils reverse transform functions and updated map images~~ Fixed plot bugs, added functionality, updated images Nov 8, 2024

Added optional argument to not plot points out of bounds of map image

2072762

Added ignore_extreme_points argument. When set to True, it will ignore points that are outside of the bounds of the default map image. Previously, code functioned as if this was set to False. Default value is False.

pnxenopoulos reviewed Nov 24, 2024

View reviewed changes

ventsiR added 2 commits November 25, 2024 17:38

Minor changes; renaming vars, adjusting function args

06fe374

Renaming of variables/functions, removal/additions of function arguments.

Renaming

60a89af

More renaming (should've pushed it with last commit)

pnxenopoulos reviewed Nov 26, 2024

View reviewed changes

Linting fixes

1bdb424

Applying linting fixes to the code

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fixed plot bugs, added functionality, updated images #353

Fixed plot bugs, added functionality, updated images #353

ventsiR commented Oct 28, 2024

pnxenopoulos left a comment

pnxenopoulos Nov 24, 2024

ventsiR Nov 25, 2024

pnxenopoulos Nov 25, 2024

ventsiR Nov 25, 2024

pnxenopoulos Nov 24, 2024

pnxenopoulos Nov 24, 2024

ventsiR Nov 25, 2024

pnxenopoulos Nov 25, 2024

pnxenopoulos Nov 26, 2024

pnxenopoulos Nov 24, 2024

ventsiR Nov 25, 2024

pnxenopoulos Nov 25, 2024

ventsiR Nov 25, 2024

pnxenopoulos Nov 24, 2024

ventsiR Nov 25, 2024 •

edited

Loading

ventsiR Nov 25, 2024

pnxenopoulos Nov 25, 2024

ventsiR Nov 25, 2024

pnxenopoulos Nov 24, 2024

ventsiR Nov 25, 2024 •

edited

Loading

pnxenopoulos Nov 25, 2024

ventsiR Nov 25, 2024 •

edited

Loading

pnxenopoulos Nov 25, 2024

pnxenopoulos commented Nov 25, 2024

pnxenopoulos Nov 26, 2024

pnxenopoulos Nov 26, 2024

pnxenopoulos Nov 26, 2024

pnxenopoulos Nov 26, 2024

		@@ -34,10 +34,40 @@ def position_transform_axis(
		return (start - position) / scale


		def position_revert_axis(

		return pixel_to_game_axis(map_name, position, axis)


		def position_transform(

Fixed plot bugs, added functionality, updated images #353

Are you sure you want to change the base?

Fixed plot bugs, added functionality, updated images #353

Conversation

ventsiR commented Oct 28, 2024

pnxenopoulos left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ventsiR Nov 25, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ventsiR Nov 25, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ventsiR Nov 25, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pnxenopoulos commented Nov 25, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ventsiR Nov 25, 2024 •

edited

Loading

ventsiR Nov 25, 2024 •

edited

Loading

ventsiR Nov 25, 2024 •

edited

Loading