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

Sort column headers for csv logger #19159

Merged
merged 6 commits into from
Dec 18, 2023

Conversation

harishb00
Copy link
Contributor

@harishb00 harishb00 commented Dec 14, 2023

What does this PR do?

This PR fixes the issue (#18978 ) where every time a csv log is written, the column headers are of random order. The fix is sorting the column headers. A new testcase is added to test the implementation.

Files affected

  • src/lightning/fabric/loggers/csv_logs.py
  • tests/tests_pytorch/loggers/test_csv.py

📚 Documentation preview 📚: https://pytorch-lightning--19159.org.readthedocs.build/en/19159/

@github-actions github-actions bot added fabric lightning.fabric.Fabric pl Generic label for PyTorch Lightning package labels Dec 14, 2023
Copy link

codecov bot commented Dec 14, 2023

Codecov Report

Merging #19159 (d6e0447) into master (119039b) will decrease coverage by 29%.
The diff coverage is 100%.

Additional details and impacted files
@@            Coverage Diff             @@
##           master   #19159      +/-   ##
==========================================
- Coverage      83%      54%     -29%     
==========================================
  Files         443      438       -5     
  Lines       36865    36768      -97     
==========================================
- Hits        30719    19924   -10795     
- Misses       6146    16844   +10698     

@awaelchli awaelchli added feature Is an improvement or enhancement community This PR is from the community logger: csv labels Dec 16, 2023
@awaelchli awaelchli added this to the 2.2 milestone Dec 16, 2023
@awaelchli awaelchli changed the title sort column headers for csv logger (bugfix) Sort column headers for csv logger (bugfix) Dec 16, 2023
@awaelchli awaelchli changed the title Sort column headers for csv logger (bugfix) Sort column headers for csv logger Dec 16, 2023
Copy link
Contributor

@carmocca carmocca left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perfect

@carmocca carmocca merged commit d4cb46d into Lightning-AI:master Dec 18, 2023
118 checks passed
@mergify mergify bot added the ready PRs ready to be merged label Dec 18, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
community This PR is from the community fabric lightning.fabric.Fabric feature Is an improvement or enhancement logger: csv pl Generic label for PyTorch Lightning package ready PRs ready to be merged
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants