-
Notifications
You must be signed in to change notification settings - Fork 97
Kunaljubce/add decorators for functions #253
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
Closed
kunaljubce
wants to merge
45
commits into
mrpowers-io:planning-1.0-release
from
kunaljubce:kunaljubce/add-decorators-for-functions
Closed
Changes from 1 commit
Commits
Show all changes
45 commits
Select commit
Hold shift + click to select a range
e642b86
Adding decoratory factory to validate_schema to make it work both as …
kunaljubce 3188b54
Fix to execute the validation when func is called and replaced the ol…
kunaljubce a353a69
Changes to tests to conform to new validate_schema definition
kunaljubce 85cc47a
Updating README description for validate_schema
kunaljubce e38ba8e
README fix
kunaljubce e58ccdd
Improved documentation in README
kunaljubce fe843b5
Added success msg to be printed in case df schema matches the require…
kunaljubce a964e16
Added a uncommitted directory for developers to store their scripts o…
kunaljubce c856f79
Minor README documentation update
kunaljubce 151dcc2
Moved uncommitted folder
kunaljubce b9a6d08
Removing uncommitted dir
kunaljubce 7c8ae16
update column extension function names and desc in readme
e9f8948
Merge pull request #240 from fatemetardasti96/main
SemyonSinchenko 21d87e5
Static type error fixes
kunaljubce 7ab9a42
Resolved merge conflicts
kunaljubce 1d33b91
Changed _df param name to df_to_be_validated and associated tests cha…
kunaljubce 2fca007
README changes for _df change
kunaljubce c4cc8af
Remove the print_athena_create_table function
nijanthanvijayakumar 5faadae
Remove deprecated functions exists and forall
nijanthanvijayakumar 0823158
Remove imported and unused Callable module to avoid ruff lint failure
nijanthanvijayakumar a9b040f
Drop Spark-2 support and update dependencies
SemyonSinchenko 60c7fb7
Update linting CI
SemyonSinchenko 5b545ef
Fix typo in CI
SemyonSinchenko e505a21
Fix failed tests
SemyonSinchenko 7802545
Updates from review
SemyonSinchenko e6ee244
Create the first-version of files for Spark-Connect tests
nijanthanvijayakumar dbd3f66
Address the fixtures issue in the test file
nijanthanvijayakumar 3ef4219
Update the CI workflow to initiate the sparkconnect test on the 1.0
nijanthanvijayakumar b1573b4
Update the poetry & pyproject with the dependencies for Spark-Connect
nijanthanvijayakumar fc85013
Update the CI workflow to run Spark-Connect tests only for v3.4+
nijanthanvijayakumar 3e8776a
Update the script to check if Spark-Connect server is running or not
nijanthanvijayakumar 8f76b0c
Remove the spark-connect server run check
nijanthanvijayakumar 0fb197e
Update workflows & pytest to choose the Sparksession instance based o…
nijanthanvijayakumar b413920
Add a TODO statement so that the spark-connect server check can be ad…
nijanthanvijayakumar f3cf717
Remove the 1.0 planning branch for the CI file
nijanthanvijayakumar 0ab7493
Attribute the original script that inspired this
nijanthanvijayakumar 3c669fc
Mark recently added deps as optional for Spark-Classic
nijanthanvijayakumar f62185f
Rename the spark-classic to connect & update makefile to install thes…
nijanthanvijayakumar 93f39d1
update column extension function names and desc in readme
b9926fd
add acknowledgement
fpgmaas 74545c3
Fix the linting issues in the linting CI workflow
nijanthanvijayakumar 943918a
remove .python-version
fpgmaas 0a71190
apply hotfix
fpgmaas ad45d31
run lint also on pr
fpgmaas f04ab78
update column extension function names and desc in readme
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
Why are we using private variables (I mean
_df) naming convention for a public API (I mean function arguments)?Uh oh!
There was an error while loading. Please reload this page.
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.
Duh! Sorry, I meant to change this and completely forgot. Let me fix this.
Meanwhile, can we not have
ruff-formatas one of the pre-commit hooks? First - It's experimental and called out so; second - it seems to be reformatting a whole lot of files which are not part of this PR when I run it on local.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.
@SemyonSinchenko Renamed
_dftodf_to_be_validatedThere 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.
@fpgmaas May you take a look, please?
Uh oh!
There was an error while loading. Please reload this page.
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.
@kunaljubce In the CI/CD pipeline I see that only 1 file is improperly formatted. I see your
.pre-commit-config.yamlcontains unstaged changes, my guess that is causing your issue. Why is that changed, and what does thepre-commit-config.yamllook like?Uh oh!
There was an error while loading. Please reload this page.
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.
@fpgmaas The unstaged changes are because I added
- id: ruff-formatto mypre-commit-config.yaml. Here's a screen recording of my changes and pre-commit succeeding beforeruff-formatv/s pre-commit failing and fixing 9 files afterruff-format- https://ufile.io/792yfg0cI could not upload the video here itself due to size restrictions.
Uh oh!
There was an error while loading. Please reload this page.
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 don't think you should edit that file manually, did you try pulling or rebasing on top of
planning-1.0-release?If I look at your branch, you are still using an outdated version of ruff, see here. That is likely causing the issue you are seeing. In order to prevent that you have to update your branch with the changes on
planning-1.0-releasefrom this repo. e.g.Uh oh!
There was an error while loading. Please reload this page.
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.
@fpgmaas That was my mistake. Out of instinct, I rebased this branch with
mainand now retrying to rebase it withplanning-1.0-releasehas corrupted this PR. 😭Closing this and reopening a fresh PR - #255.
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.
@kunaljubce Ah, that explains! Understandable mistake :) Good you were able to figure it out.