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

Wrap decorators to preserve docstrings and move to separate module #359

Merged
merged 4 commits into from
Nov 21, 2023

Conversation

w-k-jones
Copy link
Member

@w-k-jones w-k-jones commented Oct 27, 2023

Resolves #356 and reorganises decorators to a separate module

  • Have you followed our guidelines in CONTRIBUTING.md?
  • Have you self-reviewed your code and corrected any misspellings?
  • Have you written documentation that is easy to understand?
  • Have you written descriptive commit messages?
  • Have you added NumPy docstrings for newly added functions?
  • Have you formatted your code using black?
  • If you have introduced a new functionality, have you added adequate unit tests?
  • Have all tests passed in your local clone?
  • If you have introduced a new functionality, have you added an example notebook?
  • Have you kept your pull request small and limited so that it is easy to review?
  • Have the newest changes from this branch been merged?

@w-k-jones w-k-jones added this to the Version 1.5.2 milestone Oct 27, 2023
@w-k-jones w-k-jones added the documentation Updates and improvements to documentation and examples label Oct 27, 2023
@codecov
Copy link

codecov bot commented Oct 27, 2023

Codecov Report

Attention: 3 lines in your changes are missing coverage. Please review.

Comparison is base (5f096e3) 56.68% compared to head (c137f44) 56.77%.

Additional details and impacted files
@@              Coverage Diff              @@
##           RC_v1.5.x     #359      +/-   ##
=============================================
+ Coverage      56.68%   56.77%   +0.08%     
=============================================
  Files             19       20       +1     
  Lines           3426     3433       +7     
=============================================
+ Hits            1942     1949       +7     
  Misses          1484     1484              
Flag Coverage Δ
unittests 56.77% <96.70%> (+0.08%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Coverage Δ
tobac/utils/bulk_statistics.py 82.75% <100.00%> (ø)
tobac/utils/internal/basic.py 80.73% <100.00%> (-6.51%) ⬇️
tobac/utils/periodic_boundaries.py 97.00% <100.00%> (ø)
tobac/utils/decorators.py 96.51% <96.51%> (ø)

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

@w-k-jones w-k-jones added the enhancement Addition of new features, or improved functionality of existing features label Oct 27, 2023
@w-k-jones
Copy link
Member Author

Note that I wasn't able to use functools.wrap with the njit_if_available decorator because it works a bit differently to usual. If you have any ideas @freemansw1 let me know, but given that it's only used on internal functions I don't think the lack of docstrings when calling help is a problem, unlike with the xarray conversion decorators which wrap the main user functions

@JuliaKukulies JuliaKukulies mentioned this pull request Nov 8, 2023
8 tasks
@freemansw1
Copy link
Member

freemansw1 commented Nov 8, 2023

Note that I wasn't able to use functools.wrap with the njit_if_available decorator because it works a bit differently to usual. If you have any ideas @freemansw1 let me know, but given that it's only used on internal functions I don't think the lack of docstrings when calling help is a problem, unlike with the xarray conversion decorators which wrap the main user functions

My guess is that this is done automagically through Numba because help returns the correct docstring; when I return func the correct docstring is also reported.

Copy link
Member

@freemansw1 freemansw1 left a comment

Choose a reason for hiding this comment

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

Thanks for doing this, @w-k-jones ! I think you'll need to update before merge given #341 being merged, but I'm happy for you to merge after doing so.

@JuliaKukulies
Copy link
Member

JuliaKukulies commented Nov 11, 2023

Many thanks for this one, @w-k-jones! I went ahead and fixed the merge conflicts. When everything looks fine for you, please go ahead and merge :)

@freemansw1 freemansw1 linked an issue Nov 13, 2023 that may be closed by this pull request
@JuliaKukulies
Copy link
Member

Ready to merge @w-k-jones @freemansw1 ? :)

@w-k-jones w-k-jones merged commit 5518611 into tobac-project:RC_v1.5.x Nov 21, 2023
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Updates and improvements to documentation and examples enhancement Addition of new features, or improved functionality of existing features
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Use functools.wraps to include docstrings in decorated functions
3 participants