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

Add example to convert file formats and changing keypoints #304

Merged

Conversation

Lauraschwarz
Copy link
Contributor

@Lauraschwarz Lauraschwarz commented Sep 12, 2024

Before submitting a pull request (PR), please read the contributing guide.

Please fill out as much of this template as you can, but if you have any problems or questions, just leave a comment and we will help out :)

Description

What is this PR
this PR adds functions to rename, delete and re-order keypoints in your pose-estimation files.

  • Bug fix
  • Addition of a new feature
  • Other

Why is this PR needed?
It just adds flexibility to your pose-estimation files. sometimes you would like to rename or delete keypoints without having to go through the entire prediction process of DLC or SLEAP
What does this PR do?
it adds functions to rename, re-order and delete keypoints in you post processing

How has this PR been tested?

locally on my own data.
Please explain how any new code has been tested, and how you have ensured that no existing functionality has changed.
it is additional functionality and does not change any of the functionality that currently exists.

Is this a breaking change?

no

Does this PR require an update to the documentation?

no (it is in itself an update to the docs)

Checklist:

  • The code has been tested locally
  • The documentation has been updated to reflect any changes
  • The code has been formatted with pre-commit

Copy link

codecov bot commented Sep 12, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 99.77%. Comparing base (b399ce0) to head (991ba1d).

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #304   +/-   ##
=======================================
  Coverage   99.77%   99.77%           
=======================================
  Files          15       15           
  Lines         909      909           
=======================================
  Hits          907      907           
  Misses          2        2           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@niksirbi niksirbi linked an issue Sep 12, 2024 that may be closed by this pull request
@niksirbi niksirbi added the documentation Improvements or additions to documentation label Sep 12, 2024
@Lauraschwarz Lauraschwarz marked this pull request as ready for review September 20, 2024 09:54
Copy link
Member

@niksirbi niksirbi left a comment

Choose a reason for hiding this comment

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

Thanks a lot for you contribution @Lauraschwarz!

Don't be alarmed by the number of comments, most of them have to do with enforcing a consistent style across our examples, and as such may appear nitpicky. But stylistic consistency pays off for the readers (I hope).

Let me know if you have questions regarding my suggestions, or if you need help with their implementation.

examples/convert_file_formats.py Outdated Show resolved Hide resolved
examples/convert_file_formats.py Outdated Show resolved Hide resolved
examples/convert_file_formats.py Outdated Show resolved Hide resolved
examples/convert_file_formats.py Outdated Show resolved Hide resolved
examples/convert_file_formats.py Outdated Show resolved Hide resolved
examples/convert_file_formats.py Outdated Show resolved Hide resolved
examples/convert_file_formats.py Outdated Show resolved Hide resolved
examples/convert_file_formats.py Outdated Show resolved Hide resolved
examples/convert_file_formats.py Outdated Show resolved Hide resolved
examples/convert_file_formats.py Outdated Show resolved Hide resolved
Copy link

sonarcloud bot commented Oct 22, 2024

Copy link
Member

@niksirbi niksirbi left a comment

Choose a reason for hiding this comment

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

Thanks a lot for implementing my feedback @Lauraschwarz!

I've taken the liberty of putting some finishing touches:

  • moved some contents between notebook cells, for a more uniform structure
  • used variable names and imports similar to elsewhere in movement docs
  • slightly edited language (e.g. tenses) to make it consistent throughout
  • added hyperlinks to relevant pages of the movement and xarray docs (for further reading)
  • I clarified the function of the split_individuals argument for saving files.
  • In the final convert_all() function, I replaced return with continue in the case that dest_path already exists. That's because I think your intention is to skip that particular file in the loop, not to exit the entire function for the sake of 1 file.
  • I added a default thumbnail image for examples such as yours, which lack a plot (otherwise the first plot is automatically used as a thumbnail).
  • I rebased the PR to main to ensure nothing it outdated.

Assuming all tests pass, I will merge this PR.

When we make the next PyPI release, this example will be automatically published on our website 🎉 . Thanks for your hard work.

@niksirbi niksirbi added this pull request to the merge queue Oct 22, 2024
Merged via the queue into neuroinformatics-unit:main with commit 6c32608 Oct 22, 2024
17 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation
Projects
Development

Successfully merging this pull request may close these issues.

Document renaming of keypoints and individuals
2 participants