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.

Sign up for GitHub

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

Jump to bottom

Remove all datasets related extras_require and move requirements to pyproject.toml #3078

Merged
merged 24 commits into from
Oct 3, 2023
Merged

Remove all datasets related extras_require and move requirements to pyproject.toml #3078

merged 24 commits into from
Oct 3, 2023

Conversation

ankatiyar
Copy link
Contributor

@ankatiyar ankatiyar commented Sep 25, 2023 •
edited
Loading

NOTE: Kedro datasets are moving from kedro.extras.datasets to a separate kedro-datasets package in
kedro-plugins repository. Any changes to the dataset implementations
should be done by opening a pull request in that repository.

Description

Resolve #2127

Development notes

  • Moved all test and docs requirement to pyproject.toml from setup.py
  • There's some package_data stuff left in setup.py which can be moved as a follow up and we can remove setup.py entirely
  • Had to update some tests to make them run without kedro.extras.datasets
  • pandas.util.testing was deprecated, so updated it.

Checklist

  • Read the contributing guidelines
  • Signed off each commit with a Developer Certificate of Origin (DCO)
  • Opened this PR as a 'Draft Pull Request' if it is work-in-progress
  • Updated the documentation to reflect the code changes
  • Added a description of this change in the RELEASE.md file
  • Added tests to cover my changes
  • Checked if this change will affect Kedro-Viz, and if so, communicated that with the Viz team

ankatiyar and others added 14 commits September 25, 2023 12:58
@ankatiyar
Remove some requirements from setup.py
b1c1bd9 
Signed-off-by: Ankita Katiyar <ankitakatiyar2401@gmail.com>
@ankatiyar
Comment out dependencies
e7ad050 
Signed-off-by: Ankita Katiyar <ankitakatiyar2401@gmail.com>
@ankatiyar
Try fixing tests
bd4d424 
Signed-off-by: Ankita Katiyar <ankitakatiyar2401@gmail.com>
@ankatiyar
Add s3fs back
1aa3fb2 
Signed-off-by: Ankita Katiyar <ankitakatiyar2401@gmail.com>
@ankatiyar
Add pyarrow back
8e325d4 
Signed-off-by: Ankita Katiyar <ankitakatiyar2401@gmail.com>
@ankatiyar
Fix test
2b3ca1a 
Signed-off-by: Ankita Katiyar <ankitakatiyar2401@gmail.com>
@ankatiyar
Try fixing test
4ba1511 
Signed-off-by: Ankita Katiyar <ankitakatiyar2401@gmail.com>
@ankatiyar
Fix e2e test
36385cd 
Signed-off-by: Ankita Katiyar <ankitakatiyar2401@gmail.com>
@ankatiyar
Fix unit test?
57883e8 
Signed-off-by: Ankita Katiyar <ankitakatiyar2401@gmail.com>
@ankatiyar
Merge branch 'develop' into remove-extras-require
7762f40
@ankatiyar
Move requirements from setup.py to pyproject.toml
8cd072a 
Signed-off-by: Ankita Katiyar <ankitakatiyar2401@gmail.com>
@ankatiyar
Revert cloud version test
6a84af5 
Signed-off-by: Ankita Katiyar <ankitakatiyar2401@gmail.com>
@ankatiyar
Add package data to pyproject.toml
a26a779 
Signed-off-by: Ankita Katiyar <ankitakatiyar2401@gmail.com>
@ankatiyar
Revert to setup.py
0aa7100 
Signed-off-by: Ankita Katiyar <ankitakatiyar2401@gmail.com>
ankatiyar and others added 3 commits September 26, 2023 14:30
@ankatiyar
Try fixing unit test
f9c131f 
Signed-off-by: Ankita Katiyar <ankitakatiyar2401@gmail.com>
@ankatiyar
Merge branch 'develop' into remove-extras-require
0920875
@ankatiyar
Fix unit test
d28b0bc 
Signed-off-by: Ankita Katiyar <ankitakatiyar2401@gmail.com>
@ankatiyar ankatiyar marked this pull request as ready for review September 26, 2023 15:23
@ankatiyar ankatiyar requested a review from merelcht as a code owner September 26, 2023 15:23
@ankatiyar ankatiyar changed the title Remove all datasets related extras_require Remove all datasets related extras_require and move requirements to pyproject.toml Sep 26, 2023
merelcht
merelcht reviewed Sep 27, 2023
View reviewed changes
Copy link
Member

@merelcht merelcht left a comment

Choose a reason for hiding this comment

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

Generally looks good! Can't wait to see setup.py completely removed.

pyproject.toml Outdated Show resolved Hide resolved
ankatiyar and others added 5 commits September 27, 2023 16:27
@ankatiyar
Revert changes from individual e2e test and change environment.py
79be7f7 
Signed-off-by: Ankita Katiyar <ankitakatiyar2401@gmail.com>
@ankatiyar
Remove jupyter upper bounds
53d6db2 
Signed-off-by: Ankita Katiyar <ankitakatiyar2401@gmail.com>
@ankatiyar
Merge branch 'develop' into remove-extras-require
95c690b
@ankatiyar
Pandas version
77c1d72 
Signed-off-by: Ankita Katiyar <ankitakatiyar2401@gmail.com>
@ankatiyar
Pin fsspec version in test requirements
857eb07 
Signed-off-by: Ankita Katiyar <ankitakatiyar2401@gmail.com>
@ankatiyar
lint
fbd1579 
Signed-off-by: Ankita Katiyar <ankitakatiyar2401@gmail.com>
merelcht
merelcht approved these changes Oct 2, 2023
View reviewed changes
Copy link
Member

@merelcht merelcht left a comment

Choose a reason for hiding this comment

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

LGTM! 👍

pyproject.toml
"blacken-docs==1.9.2",
"black~=22.0",
"coverage[toml]",
"fsspec<2023.9", # Temporary, newer version causing "test_no_versions_with_cloud_protocol" to fail
Copy link
Member

Choose a reason for hiding this comment

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

Would be good to have a follow up ticket to unpin this again.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yep, I'll create one.

@ankatiyar ankatiyar mentioned this pull request Oct 2, 2023
Closed
astrojuanlu
astrojuanlu approved these changes Oct 3, 2023
View reviewed changes
Copy link
Member

@astrojuanlu astrojuanlu left a comment

Choose a reason for hiding this comment

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

Looks good to me! Thanks @ankatiyar ⭐

setup.py
SPARK = "pyspark>=2.2, <3.4"
HDFS = "hdfs>=2.5.8, <3.0"
S3FS = "s3fs>=0.3.0, <0.5"

template_files = []
for pattern in ["**/*", "**/.*", "**/.*/**", "**/.*/.**"]:
template_files.extend(
Copy link
Member

Choose a reason for hiding this comment

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

I'm so looking forward to deleting this too 😄 (in a future PR)

@ankatiyar ankatiyar merged commit 145f91f into develop Oct 3, 2023
28 of 29 checks passed
@ankatiyar ankatiyar deleted the remove-extras-require branch October 3, 2023 12:34
@ankatiyar ankatiyar mentioned this pull request Oct 3, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@noklam noklam noklam left review comments

@AhdraMeraliQB AhdraMeraliQB AhdraMeraliQB approved these changes

@merelcht merelcht merelcht approved these changes

@astrojuanlu astrojuanlu astrojuanlu approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@ankatiyar @astrojuanlu @noklam @merelcht @AhdraMeraliQB