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

Don't sort on groupby (e.g. in skill() method) #376

Merged
merged 29 commits into from
Jan 8, 2024
Merged

Conversation

jsmariegaard
Copy link
Member

@jsmariegaard jsmariegaard commented Jan 4, 2024

  • groupby(..., sort=False) in skill() and similar methods
    • correct tests accordingly
  • Fix bug in weights when assigning from dict (caused wrong results in some cases)
  • Re-introduce to_dataframe() on Comparer
  • Syncronize to_dataframe() of Comparer and Timeseries such that they work in the same way (include aux also)
  • Docstrings for all to_dataframe() methods
  • Docstrings for several other methods
  • Renaming of internal variables (and variables in tests) to confirm to best practice and for consistency (e.g. sk for skill() )
  • New method sort_values() on SkillTable
  • xy set to nan for skill() of track data

@jsmariegaard jsmariegaard changed the title Don't sort on groupby (e.g. from skill() method) Don't sort on groupby (e.g. in skill() method) Jan 5, 2024
@jsmariegaard jsmariegaard marked this pull request as ready for review January 5, 2024 10:04
@jsmariegaard
Copy link
Member Author

jsmariegaard commented Jan 7, 2024

Now added more tests. Okay to merge @ecomodeller ?

@ecomodeller
Copy link
Member

Now added more tests. Okay to merge @ecomodeller ?

Good, and I changed the _to_long_dataframe a bit more.

@jsmariegaard jsmariegaard merged commit 64e17dd into main Jan 8, 2024
10 checks passed
@jsmariegaard jsmariegaard deleted the groupby-sort-false branch January 8, 2024 09:56
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.

2 participants