-
Notifications
You must be signed in to change notification settings - Fork 98
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 decorators for functions #221
Add decorators for functions #221
Conversation
…a function and as a decorator
…d func definition of validate_schema()
Is it a breaking change, isn't it? If so, I would suggest pushing it into 1.0, not main. In the same moment, we need to mark these functions as deprecated in the main and make a new release. @MrPowers what do you think? |
Could be a potential breaking change, yes. Would like to discuss more on this. Let me move this to 1.0. |
I don't see a branch for 1.0. Did we create it? I remember seeing a label for 1.0, though. |
@SemyonSinchenko Also we are not exactly deprecating this functionality but evolving it to serve a better purpose, if I may say so 😆 Would a I would think a better way would be to publish this change under a change log or something. |
…r code snippets relevant to quinn without making them visible to git
Implementation details for validate_schemaSo basically we are implementing a decorator factory here so that our function can be used both as a decorator as well as a callable function. In this implementation:
When |
Hmm, maybe it would be better to leave a current function as is and create a new one? I don't know. @MrPowers @jeffbrennan, what do you think, guys? |
I like the idea of having one function that uses the decorator if no dataframe is provided - we would need to keep the arguments consistent with the current |
@jeffbrennan The only difference in the arguments is the absence of |
@SemyonSinchenko This was the case with #141. I agree with the discussion in there that two different functions achieving a similar objective, one being used as a callable and one being used as a decorator can be confusing to users. That, in fact, was my inspiration to take a fresh attempt at this problem. |
@SemyonSinchenko @jeffbrennan @MrPowers Closing this and have raised PR #253 to merge to 1.0-release branch |
Proposed changes
Took another stab at #140, as an extension to #144.
Types of changes
What types of changes does your code introduce to quinn?
Put an
x
in the boxes that apply