-
-
Notifications
You must be signed in to change notification settings - Fork 236
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鈥檒l occasionally send you account related emails.
Already on GitHub? Sign in to your account
ENH: reimplement plotting module #585
base: master
Are you sure you want to change the base?
Conversation
Amazing work! 馃檹馃徎 Any specific reason why the On a side note (and very low priority), a tricky question comes to mind about 'Official' team colors. From the documentation:
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. |
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 馃槄
You have read the code for
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. |
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 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:
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
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) 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. |
@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. |
That needs to be fixed, true. There really shouldn't be warnings in those cases.
I think that's a reasonable suggestion, including the renaming of the colormaps. And it shouldn't be too difficult to do.
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. |
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. |
That looks very good. I especially love the way how we can combine (partial) custom colouring styles with the default style in This is related to "3. Support for changing constants (colors, names)" above. Can we pass a dictionary to fastf1.plotting.setup_mpl(color_scheme='fastf1',
custom_scheme={
'team': {
'alpine': '#000000'
},
'driver': {
'ver': '#123456',
'pez': '#ABCDEF'
}
}) |
This pull request fully reimplements the
fastf1.plotting
submodule with the following aims:Summary of the changes:
.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..get_driver_style()
function can be used to largely automate this.Deprecations:
driver_color
,lapnumber_axis
,team_color
COMPOUND_COLORS
,DRIVER_TRANSLATE
,DRIVER_COLORS
,TEAM_COLORS
,TEAM_TRANSLATE
,COLOR_PALETTE
misc_mpl_mods
of function.setup_mpl()
is deprecated and will be removed without replacement.Upcoming behaviour changes:
color_scheme
of function.setup_mpl()
will default toNone
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.0a0
To-Do
exact_match
parameter to all functions that use fuzzy matchingget_driver_style
in documentation