Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ENH: reimplement plotting module #585

Merged
merged 19 commits into from
Jul 24, 2024
Merged

ENH: reimplement plotting module #585

merged 19 commits into from
Jul 24, 2024

Conversation

theOehrly
Copy link
Owner

@theOehrly theOehrly commented May 12, 2024

This pull request fully reimplements the fastf1.plotting submodule with the following aims:

  • increase compatibility to all drivers/teams/session from 2018 to the current season
  • reduce maintenance effort

Summary of the changes:

  • Team colours and short team names are hard-coded per season to support abbreviated team names and higher contrast colour maps that aren't available from the APIs
  • Driver names, driver abbreviations and the driver-to-team mapping is loaded from the F1 livetiming API for each session. This way, driver changes, test drivers, ... are automatically supported.
  • The existing lists/dictionaries of driver colours/team colours/driver abbreviations/... are deprecated and replaced with new API consisting of .get_<something>() functions. This serves two purposes. First, it decouples the internal implementation for providing the data from the public API. Second, it should make for more concise and readable code in almost all use-cases.
  • Driver-specific colours are deprecated. The driver colour will now identically match the team colour for all drivers of a team. To differentiate drivers in graphics, other stylistic options should be used. A new .get_driver_style() function can be used to largely automate this.
  • The old API is fully supported during the deprecation period.

Deprecations:

  • functions driver_color, lapnumber_axis, team_color
  • attributes COMPOUND_COLORS, DRIVER_TRANSLATE, DRIVER_COLORS, TEAM_COLORS, TEAM_TRANSLATE, COLOR_PALETTE
  • argument misc_mpl_mods of function .setup_mpl() is deprecated and will be removed without replacement.

Upcoming behaviour changes:

  • Argument color_scheme of function .setup_mpl() will default to None instead of 'fastf1' in the future, meaning that FastF1's default colour scheme needs to be enabled explicitly.

I appreciate reviews/comments/testing of these changes, even if comments/testing is just limited to parts of this (fairly large) change 😄 (maybe @Casper-Guo, @pesaventofilippo, @harningle ?)

To facilitate this, here is a link to a prebuilt version of the documentation: https://theoehrly.github.io/Fast-F1-Pre-Release-Documentation/plotting.html

For easy testing with your existing scripts, a pre-release is available through PyPI and can be installed with pip install fastf1==3.4.0a1

To-Do

  • add exact_match parameter to all functions that use fuzzy matching
  • evaluate option to automatically support future seasons in a limtied way (no 'default' colormap, no short team names) deferred, TODO: create issue, make this a future problem
  • fix formatting of code example for get_driver_style in documentation
  • refactor fuzzy matching, fix lookup warnings (problems see ENH: reimplement plotting module #585 (comment))
  • setting default color map and names of color maps as suggested in ENH: reimplement plotting module #585 (comment)
  • option for overriding constants, probably use implementation similar to suggestion in ENH: reimplement plotting module #585 (comment)
  • try to generate official color map dynamically from API

@pesaventofilippo
Copy link
Contributor

Amazing work! 🙏🏻
I've read the new documentation and it feels very solid and well-organized, especially the driver style functions.
I tested it quickly and it works very well, but I plan on testing (almost) all changes more in depth ASAP.

Any specific reason why the misc_mpl_mods argument will be removed? They seem quite "fixed" changes to mpl, in the sense that they shouldn't need to be maintained or modified over time. If (and only if) that is the case, then maybe a good idea would be to keep that functionality, maybe by moving it in a separate function like .enable_misc_mpl_mods()?

On a side note (and very low priority), a tricky question comes to mind about 'Official' team colors. From the documentation:

Colors are constant over the course of a season.

If I recall correctly, at the start of 2024 F1 used pink as Alpine's color, and after a while the official color changed to blue-ish, which is the current color inside of the Constants file.
Let's say a new season starts, teams' colors get published and so the initial colors are picked as the Official palette. If later in the season F1 changes color for a team, what takes precedence, using the latest official color or keeping them constant over a season?
If I were to choose I'd stay with the first color (also to avoid having to update the constants mid-season), but then the 'Official' palette and the team color returned by the API would differ, so for clarity it should be specified in the docs 👀

@theOehrly
Copy link
Owner Author

Amazing work! 🙏🏻 I've read the new documentation and it feels very solid and well-organized, especially the driver style functions. I tested it quickly and it works very well, but I plan on testing (almost) all changes more in depth ASAP.

Thanks 😃 You don't need to hurry too much to test these changes. After all, it has taken me about a quarter year longer than planned to get here. After I prioritized other things over this for more than a year already. A few days or weeks won't matter too much, I guess 😅

Any specific reason why the misc_mpl_mods argument will be removed? They seem quite "fixed" changes to mpl, in the sense that they shouldn't need to be maintained or modified over time. If (and only if) that is the case, then maybe a good idea would be to keep that functionality, maybe by moving it in a separate function like .enable_misc_mpl_mods()?

You have read the code for misc_mpl_mods? There are a bunch of things that I don't like about it.
First, it's neither really self explaining nor is it commented. I actually don't know what it really does. Second, it patches a bunch of functions from Matplotlib's public API and replaces them with customized versions. Third, it does all this to various functions, doing various things, which makes it very difficult to document properly.
In its current state, no one really knows what changes/happens, and that's not really great for the users. It's just a function that is called in some examples, and it happens to make some changes to the plot that are apparently desired in the example. But it's unclear what these changes are. I don't think that FastF1 should modify the behaviour of other libraries in such a drastic and unpredictable manner. Especially not, when users could just make these changes themselves explicitly.

On a side note (and very low priority), a tricky question comes to mind about 'Official' team colors. From the documentation:

Colors are constant over the course of a season.

If I recall correctly, at the start of 2024 F1 used pink as Alpine's color, and after a while the official color changed to blue-ish, which is the current color inside of the Constants file. Let's say a new season starts, teams' colors get published and so the initial colors are picked as the Official palette. If later in the season F1 changes color for a team, what takes precedence, using the latest official color or keeping them constant over a season? If I were to choose I'd stay with the first color (also to avoid having to update the constants mid-season), but then the 'Official' palette and the team color returned by the API would differ, so for clarity it should be specified in the docs 👀

That's actually a fairly annoying problem to solve, but we should think about it. The first question is, what would be the desired behaviour? Do we want to have constant team colours? Then we need to make some kind of rule for which colour is used. Otherwise, it would be possible to create the 'official' colour map dynamically from the API. I need to think about this.

@pesaventofilippo
Copy link
Contributor

pesaventofilippo commented May 13, 2024

After using it a bit more, I have some feedback, but they all are nice-to-have suggestions, so I'm open to discussions and maybe input from others!

1. Lookup warnings when getting team names from session.laps
If you need to get the short team name or color from a Lap, you would get the lap.Team, and then use that as the identifier for plotting.get_team_name. The problem is, lap.Team contains the full name of each team, while FastF1 uses the short names, so every time you lookup a team like "Haas F1 Team", "Kick Sauber" or "Red Bull Racing", fuzzy matching is used and a warning is generated, which can be annoying.

Code example:

import fastf1
from fastf1 import plotting

session = fastf1.get_session(2024, "Miami", "Q")
session.load()

for teamName, laps in session.laps.groupby('Team'):
    shortName = plotting.get_team_name(teamName, session, short=True)
    print(f"{shortName} has {len(laps)} laps")

Output:

...
_base       WARNING 	Correcting invalid user input 'haas f1 team' to 'haas'.
_base       WARNING 	Correcting invalid user input 'kick sauber' to 'sauber'.
_base       WARNING 	Correcting invalid user input 'red bull racing' to 'red bull'.

The warnings are useful when passing the team name as a string, so if a user types "red ball" it gets correctly flagged. But I think an exception needs to be made for the names returned by the API.

2. Setting the default colormap used for all plotting functions
In every function that accepts the colormap parameter, the two values are 'default' and 'official'. In general, when one wants to change the colormap, that choice is made for every function call, e.g. it's very uncommon to use a color palette for a plot and then a different palette for another plot; but as of now you would need to update every call to use the new palette (which could also be a problem for the next suggestion).
Here's a quick general idea on how I would change it, although as I said some feedback would be appreciated:

  • Rename the 'default' colormap to 'fastf1', so the two colormaps are 'fastf1' and 'official'
  • Keep the 'default' option so that, by default, using colormap='default' is the same as using colormap='fastf1'
  • Create a new function plotting.set_default_colormap(colormap: str) that changes the value which gets used for the 'default' colormap.

For example:

plotting.get_driver_color(identifier, session) # no colormap specified, using 'default', gets evaluated to 'fastf1'
plotting.set_default_colormap('official')
plotting.get_driver_color(identifier, session) # no colormap specified, using 'default', gets now evaluated to 'official'

This way, you would use the 'default' argument as always, but now you also have the option to change the default behaviour. If needed, you can always choose a specific colormap by specifying 'fastf1' or 'official'.

3. Support for changing constants (colors, names)
The general idea would be to be able to specify an override for some values. For example, I like the 'official' colormap for most colors, but I would like to manually change the Alpine color to pink (the color used by the default colormap).
Another example would be to modify the short name for a team, for example "RB" gets confused with Red Bull, so I'd replace the RB ShortName with VCARB.

Now, I have no idea if there's a good way to implement this without having to specify a new custom value for each property that is now in the Constants file. Ideally the code would work like this:

# Create a new 'constant' by specifying a name and a "base" colormap, from which all values that don't get overridden are picked. The fields of the newly created `Constants` object can be overridden.
plotting.create_constants(name='custom', base='default')

# Override a team color
plotting.constants['custom'].Teams['alpine'].TeamColor = "#ff87bc"

# Override another property
plotting.constants['custom'].Teams['rb'].ShortName = "VCARB"

# Use the new constants
plotting.get_team_color(identifier, session, colormap='custom')
plotting.get_team_name(identifier, session, map='custom') # new 'map' argument like colormap, by default map='default'

I realize this is not very intuitive and it would need at least a partial rewrite of how the constants work. If someone has a better idea of how this could work or be implemented it would be nice, otherwise this is not important.

@Casper-Guo
Copy link
Contributor

@pesaventofilippo I don't have my computer so I cannot review all the code but I'm assuming the fuzzy match logic is the same as before.

If that's the case, then these warnings were added in #574 and the logic is a simpler version of what's implemented in #579. In the latter case, no warning is generated if there is an unambiguous full partial match which would cover all the false positives here.

There is also some code duplication with fuzzy matching in the plotting module. A clean solution would be to implement the more complex logic as a helper function and reuse it. I cannot work on that any time soon so feel free to call dibs.

@theOehrly I am doing some traveling so I cannot dive in until June. Then I can start a new branch and try all the new interfaces in my own use cases.

Since this is a big change, we should add a todo to rewrite examples as needed and maybe prepare a short transition guide outlining the idiomatic usages in the old vs new versions.

@theOehrly
Copy link
Owner Author

theOehrly commented May 14, 2024

1. Lookup warnings when getting team names from session.laps

That needs to be fixed, true. There really shouldn't be warnings in those cases.
@Casper-Guo the fuzzy matching logic is similar but not exactly the same as for events. I'll look into refactoring your improved logic out and use it in all places. I think that should be doable.

2. Setting the default colormap used for all plotting functions

I think that's a reasonable suggestion, including the renaming of the colormaps. And it shouldn't be too difficult to do.

3. Support for changing constants (colors, names)

That part might be really tricky. But I see how it can be useful. I don't particularly like how the constants are implemented right now in the backend. The good thing is, though, that they are in no way exposed publicly. So I can modify the implementation any time, if it doesn't fit the needs any more. With at change like the one you are proposing, this would become public API which makes things quite a bit more complicated.
In general, the implementation isn't that much of a problem. The more difficult thing is maintainability and creating a reasonable public API for this.

@theOehrly
Copy link
Owner Author

Since this is a big change, we should add a todo to rewrite examples as needed and maybe prepare a short transition guide outlining the idiomatic usages in the old vs new versions.

The examples are updated in this PR as well. Do you think a transition guide is necessary? The old API still works, but users will get FutureWarnings when using it.

@harningle
Copy link
Contributor

That looks very good. I especially love the way how we can combine (partial) custom colouring styles with the default style in examples/plot_driver_styling.py.

This is related to "3. Support for changing constants (colors, names)" above. Can we pass a dictionary to setup_mpl, so that if we specify some colours for some teams, we use it; otherwise, use the default colours. But I'm not sure what's the best practice to combine this with how constants work.

fastf1.plotting.setup_mpl(color_scheme='fastf1',
                          custom_scheme={
    'team': {
        'alpine': '#000000'
    },
    'driver': {
        'ver': '#123456',
        'pez': '#ABCDEF'
    }
})

@theOehrly
Copy link
Owner Author

I've tried to address all your great feedback now. The new pre-release is published (pip install fastf1==3.4.0a1) and the pre-release documentation that is linked in the opening comment has been updated.

The following suggestions have been addressed:

1. Official team colours may change during the seasons

One example for this is Alpine this season, where the colour changed from pink to blue. Now, the team constants have a "base" colour for the official colour. This colour value is updated from the API on a per-session basis now. Only if there were no team colours available somehow, the base colour would be kept.

2. Lookup warnings when getting team names from session.laps

The fuzzy matching code has been refactored and unified throughout FastF1. This problem should be fixed now.

3. Naming of the colour maps and setting the default colour map

As suggested, the colour maps are now named "fastf1" and "official". The "default" keyword now refers to the default colour map, which is "fastf1". The default colour map can be changed by calling fastf1.plotting.set_default_colormap. This way, it is not necessary to specify the colour map in every function call. But it is still possible to set it per function call.

4. Support for changing constants

It is now possible to override the team's short names and the colours in each colour map. Use the function fastf1.plotting.override_team_constants for this. The overrides are per session and do not persist.

I do not want to expose the internal constants API because this is currently a semi-mess. It works right now, but there are some not so great things about it and I can't gurantee that it might not require some major breaking changes at some point. As long as this is an internal API, that's fine and the current implementation is "good enough" even though I'm not very happy with it. If I were to expose this as public API, I basically commit to keeping this as is. This might really make things much more complicated down the road. Alternatively, I'd need to give this much more thought and implement it more cleanly and future proof, delaying this long overdue change even further. Therefore, I have decided to keep this internal API. One could argue that the new function simply hides the internal mess from the user but I consider this the lesser evil here.

@harningle, overriding colours on a per-driver basis will not be added for two reasons. First, this would be a fairly large change or a very hacky implementation, because driver colours do no longer exist in the new implementation. Everything is team colours and I'd need to change that just to enable these overrides. Second, the reason why driver colours are deprecated is that they end up being a huge mess requiring shades of almost the same colour for various drivers. And in the end, you can barely tell them apart. People should use other means to differentiate drivers of the same team in a visualization.

I hope I have addressed all suggestions properly and not forgotten anything. Looking forward to your comments :)

@theOehrly
Copy link
Owner Author

One more question, I had switched Red Bull over to a yellow team colour some time ago, because there are too many teams with a blueish colour. Therefore, I decided that it is much better for readability if Red Bull gets a different colour and yellow is one of their (minor) accents colours after all. I have a feeling that this change wasn't popular at all. If everybody ends up overwriting this colour back to a blue colour, I might as well just change it back. What's your opinion?

@pesaventofilippo
Copy link
Contributor

Well, what can I say, massive work! This feels much more polished and customizable now, which is a very good thing for a "plotting update". As for the internal mess, I think it was inevitable, because some data comes from the API and some is hardcoded (and customizable). I don't think you should worry too much about it, at least for the end user's perspective. Maybe it's difficult to maintain or add stuff to it, but as you said this can always change silently in the background in the future.

Regarding the Red Bull color change, here's my opinion: I think part of the reason many (at least, I) don't use it is because Red Bull is a top team, so many people are interested to see them and people are used to look for "the blue color at the front". If there's a graph which represents Ferrari vs. RB, or Mercedes vs. RB etc, the blue color is immediately obvious (because most of the time you aren't comparing a top team to Alpine or Williams for example), while the yellow color isn't immediately obvious without reading the legend (at least the first time, but habits die hard 😂).

IF that's true, then maybe this same idea could work better by changing other teams. An example would be using pink for Alpine, which also was their official color at some point (as opposed to yellow for Red Bull). I'd say also using white for Williams could work, because they had white in their logo and in the past their car was white. But this wouldn't work with a white background graph, so maybe it's a bit of a stretch.

@Casper-Guo
Copy link
Contributor

I will start migrating my code to the new API soon. I only interact with the plotting module in a few places so hopefully it will be a smooth transition.

@harningle
Copy link
Contributor

The team colour overriding looks very good to me! I used to hardcode driver colours, but now being able to change the team's colour should be enough for almost all my use cases. I will try the new plotting module later. Thank you so much!

@theOehrly
Copy link
Owner Author

@pesaventofilippo thanks for the feedback on the yellow Red Bull colour. I think your arguments are very valid. I might have looked at this too much from a general data visualization standpoint and not enough from a fan point of view. Your arguments and the general lack of adoption of this change have convinced me 😅
Alpine currently already has pink as the team colour in the 'fastf1' colour map. As you say, white for Williams is difficult because of the default white background of Matplotlib. But maybe a future enhancement could be to add specific colour maps for dark and light backgrounds as an addition to the current compromise colour map.

@formulatimer
Copy link
Contributor

Hi,

I have been trying the new plotting module. I have not found any errors and have tried to migrate my code to this new module. I use different functions to generate graphs; one of them is the minisector that indicates which driver is the fastest in each part of the track. It is made by concatenating segments, and the color of the segment indicates who was the fastest in that area of the track.

The problem arises when I plot two drivers from the same team. I tried changing the linestyle, linewidth, and transparency, but I cannot find an appropriate solution. Do you have a solution, or should I use colors not correlated with team colors?

Another question, when do you plan to launch the update?

2024British Grand Prix-Qualifying-Best Minisectors-RUS-HAM-VER_20
2024British Grand Prix-Qualifying-Best Minisectors-RUS-HAM-VER_alpha
2024British Grand Prix-Qualifying-Best Minisectors-RUS-HAM-VER_dotted

@theOehrly
Copy link
Owner Author

@formulatimer looking at the following graphic from your Twitter account, it is just as difficult to tell HAM and RUS apart as in the examples you gave above.
GR0IwZPXwAEOETz
I don't intend to hate on your work and graphics here. But this precisely underlines the point why I have decided to deprecate driver specific colours. They are annoying to maintain, make things quite a bit more complicated, and in the end they are not a good tool to differentiate between drivers of the same team. (To my knowledge) I am not colour-blind and the monitor that I am currently viewing this on has decent colour accuracy. Yet, I am barely able to tell the colours apart here.

One good solution is to use alternate colours similar to here:
grafik

Or a different suggestion from me is to do something like this:
grafik
(This is not actually a mini sector comparison because I had not code for that ready. It's just the track segmented by gear shifts.)

@theOehrly
Copy link
Owner Author

Another question, when do you plan to launch the update?

I was considering this week. But I am still waiting on some additional feedback from someone who hasn't commented here. And a few people here wanted to test with their scripts. We'd be getting very close to the race weekend then, and I don't want to release this on a friday during a race weekend.
I'd say next week seems reasonable for a release if there are no unexpected things.

@formulatimer

This comment was marked as off-topic.

@theOehrly

This comment was marked as off-topic.

@formulatimer

This comment was marked as off-topic.

@theOehrly

This comment was marked as off-topic.

@theOehrly
Copy link
Owner Author

I will start migrating my code to the new API soon. I only interact with the plotting module in a few places so hopefully it will be a smooth transition.

@Casper-Guo have you had time to do that already? If yes, have any problems come up? Else I'd like to merge this soon.

@Casper-Guo
Copy link
Contributor

Sorry I forgot to update you. I had few points of access to the plotting module so it was fairly painless.

I was using:

  1. setup_mpl with misc_mpl_mods=False so straightforward here
  2. team_color with easy migration to get_team_color
  3. DRIVER_TRANSLATE and DRIVER_COLORS. On that front I need to figure out how to differentiate drivers from the same team using linestyles etc. but that's outside of the scope of this PR

So nothing unexpected

@theOehrly
Copy link
Owner Author

@Casper-Guo great, thanks for the feedback

@theOehrly theOehrly merged commit c56b044 into master Jul 24, 2024
18 of 20 checks passed
@theOehrly theOehrly deleted the plotting-opt2 branch July 30, 2024 15:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants