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 dprint shortcut to FunctionGraph and Function #779

Merged
merged 1 commit into from
Jul 3, 2024

Conversation

ricardoV94
Copy link
Member

Description

Follow-up to #729

----------
kwargs:
Optional keyword arguments to pass to debugprint function.
"""
Copy link
Member

Choose a reason for hiding this comment

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

In general do you know of an elegant way to copy over docstrings? It'd be nice if we could see the full dprint docstring, but only if we don't have to re-copy it in every helper method.

Copy link
Member Author

Choose a reason for hiding this comment

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

I don't know a way but I suspect there might be one.

Copy link
Member Author

Choose a reason for hiding this comment

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

Internet shows stuff with assigning .__doc__ variables. Feels a bit hacky to me but maybe it's conventional. Checking if @OriolAbril has any input here

Copy link
Member

Choose a reason for hiding this comment

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

I have seen and used copying things over and setting the .__doc__ attribute manually, I don't think there is anything wrong with it

Copy link

codecov bot commented May 24, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 80.85%. Comparing base (5f374db) to head (93a7543).
Report is 224 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##             main     #779   +/-   ##
=======================================
  Coverage   80.85%   80.85%           
=======================================
  Files         162      162           
  Lines       47045    47051    +6     
  Branches    11514    11514           
=======================================
+ Hits        38039    38045    +6     
- Misses       6750     6753    +3     
+ Partials     2256     2253    -3     
Files with missing lines Coverage Δ
pytensor/compile/function/types.py 79.68% <100.00%> (+0.08%) ⬆️
pytensor/graph/fg.py 89.17% <100.00%> (+0.09%) ⬆️

... and 3 files with indirect coverage changes

Comment on lines +1109 to +1120
def dprint(self, **kwargs):
"""Debug print itself

Parameters
----------
kwargs:
Optional keyword arguments to pass to debugprint function.
"""
from pytensor.printing import debugprint

return debugprint(self, **kwargs)

Copy link
Contributor

Choose a reason for hiding this comment

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

I have mixed feelings about this solution since this paradigm is quite rare, but perhaps it shouldn't be.

Suggested change
def dprint(self, **kwargs):
"""Debug print itself
Parameters
----------
kwargs:
Optional keyword arguments to pass to debugprint function.
"""
from pytensor.printing import debugprint
return debugprint(self, **kwargs)
from pytensor.printing import debugprint

I think this does everything cleanly, but I haven't tested it.

Copy link
Member Author

@ricardoV94 ricardoV94 Jun 4, 2024

Choose a reason for hiding this comment

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

Does this call the function with self as the first argument?

Copy link
Contributor

Choose a reason for hiding this comment

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

On class instances, yes.

Copy link
Member Author

Choose a reason for hiding this comment

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

Thats 🤯 haha

@ricardoV94
Copy link
Member Author

I'm going with the simpler approach here. If anybody wants to tweak docstrings or use @maresb trick feel free to open a new PR

@ricardoV94 ricardoV94 merged commit df769f6 into pymc-devs:main Jul 3, 2024
56 checks passed
@ricardoV94 ricardoV94 deleted the everyone_gets_dprint branch July 3, 2024 12:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants