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

[CLN] Collection of code clean-up and improvement tasks #346

Open
4 of 5 tasks
theOehrly opened this issue Apr 5, 2023 · 5 comments
Open
4 of 5 tasks

[CLN] Collection of code clean-up and improvement tasks #346

theOehrly opened this issue Apr 5, 2023 · 5 comments
Labels
CI Issue is related to CI enhancement New feature or request style Issue is related to code style

Comments

@theOehrly
Copy link
Owner

theOehrly commented Apr 5, 2023

This issue is intended for keeping track of some code clean-up and improvement tasks that are not immediately required but should be done at some point.

In no particular order:

  • make all remaining unchanged lines conform to the new 79 character line length limit
  • reconsider the import structure and style to make it more uniform
  • move major explanatory parts of the documentation to rst files; limit documentation in py files to docstrings
  • add CI tests for building FastF1 and testing the built package (catch things like missing packages in setup.cfg)
  • fix docstrings for various Laps.pick_* functions (confusing naming of vars in examples)
@theOehrly theOehrly added enhancement New feature or request style Issue is related to code style labels Apr 5, 2023
@theOehrly theOehrly added this to the v3.1.0 milestone Apr 5, 2023
@theOehrly theOehrly removed this from the v3.1.0 milestone May 7, 2023
@theOehrly theOehrly added the CI Issue is related to CI label May 7, 2023
@Casper-Guo
Copy link
Contributor

What docstring improvements do you have in mind? I am already reworking some of those functions for #376 so I can take care of that as well.

@theOehrly
Copy link
Owner Author

What docstring improvements do you have in mind? I am already reworking some of those functions for #376 so I can take care of that as well.

The short example code snippet in many Laps.pick_* functions use a weird naming convention. For example some_drivers_laps = ff1.pick_teams(['Mercedes', 'Williams']) , but ff1 is commonly used as abbreviation for the module, naming it laps would make more sense. This might be confusing, especially for newcomers.

@Casper-Guo
Copy link
Contributor

Casper-Guo commented May 25, 2023 via email

@Casper-Guo
Copy link
Contributor

With regards to input structure/style, have you investigated adding isort to pre-commit?

@theOehrly
Copy link
Owner Author

theOehrly commented Jun 15, 2023

With regards to input structure/style, have you investigated adding isort to pre-commit?

Yes, I probably want to add flake8-isort or flake8-import-order.

But I want "fix" all existing imports before that.

And I want to look into what a reasonable module structure is, to prevent circular imports (A problem that I occassionally have). This is not directly related but interesting for potential future additions and refactoring.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CI Issue is related to CI enhancement New feature or request style Issue is related to code style
Projects
None yet
Development

No branches or pull requests

2 participants