-
Notifications
You must be signed in to change notification settings - Fork 43
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
chore: unimplemented API tracking #1269
base: main
Are you sure you want to change the base?
Conversation
self.method_name, | ||
args, | ||
kwargs, | ||
task=PANDAS_API_TRACKING_TASK, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm a bit confused by this. Why the extra task
argument?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Task is to identify why the labels are collected. The PANDAS_API_TRACKING_TASK means the method is missing(like df.resample). The PANDAS_PARAM_TRACKING_TASK means args or kwargs are missing, but the method has already being implemented. Later in dashboard this may help.
) | ||
|
||
|
||
def submit_pandas_labels( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please add a docstring explaining the purpose of this method.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated
@@ -30,6 +37,78 @@ | |||
_call_stack: List = [] | |||
|
|||
|
|||
class UnimplementedMethodLogger: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pease add a docstring.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated
return method(*args, **kwargs) | ||
return method(self, *args, **kwargs) | ||
except (NotImplementedError, TypeError) as e: | ||
if hasattr(self, "_block") and len(_call_stack) == 1: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please add a comment explaining why we have these requirements in the if statement.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated
'pandas.DataFrame.{key}'. {constants.FEEDBACK_LINK} | ||
""" | ||
) | ||
return log_adapter.UnimplementedMethodLogger( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm confused. Why would we ever return this class to the user?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Return this is because gettattr won't get the args and kwargs. By returning the class, the class will be called as a function and receive the args and kwargs, and then we can set the labels.
'pandas.Series.{key}'. {constants.FEEDBACK_LINK} | ||
""" | ||
) | ||
return log_adapter.UnimplementedMethodLogger( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Likewise, returning here seems wrong.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Return this is because gettattr won't get the args and kwargs. By returning the class, the class will be called as a function and receive the args and kwargs, and then we can set the labels.
Thank you for opening a Pull Request! Before submitting your PR, there are a few things you can do to make sure it goes smoothly:
Currently this covers Series class and DataFrame class
Fixes #<issue_number_goes_here> 🦕