feat: make pandas an optional dependency (#35)#49
feat: make pandas an optional dependency (#35)#49chenghao-guo merged 1 commit intolance-format:mainfrom
Conversation
|
ACTION NEEDED The PR title and description are used as the merge commit message. Please update your PR title and description to match the specification. For details on the error please inspect the "PR Title Check" action. |
Gurukiran20
changed the title
| # Marimo | ||
| marimo/_static/ | ||
| marimo/_lsp/ | ||
| __marimo__/ |
There was a problem hiding this comment.
Please do not change the .gitignore. Otherwise, there are a lot of dirty dependency code injected, which is not related with lance-ray.
There was a problem hiding this comment.
all myenv/ folder should not be commited
|
Hi @chenghao-guo sir,
The PR has been updated accordingly. Please re-review it sir. |
|
These are also not required For the ut failure, probably we need to make pandas as a test dependency. So probably we put it in test for ut pass, and not required for lance-ray itself |
|
Hi @chenghao-guo sir, and @jackye1995 I've updated the PR as requested:
The PR has been updated; however I'm still seeing a test failure related to Ray internal modules ( Could you please help with this test failure? The changes requested in the code review have all been addressed. 😊 |
README.md
Outdated
| - [Apache Arrow](https://arrow.apache.org/) for in-memory data structures | ||
| - [User Guide and API Documentation](https://lancedb.github.io/lance/integrations/ray/) | ||
| - [Contributing Guide and Dev Setup](./CONTRIBUTING.md) | ||
|
|
There was a problem hiding this comment.
Why changing a lot of unnecessary document in this md ? Is the code not rebased?
lance_ray.egg-info/SOURCES.txt
Outdated
| @@ -0,0 +1,13 @@ | |||
| LICENSE | |||
There was a problem hiding this comment.
all lance_ray.egg-info should not be included
docs/contributing.md
Outdated
| @@ -0,0 +1,39 @@ | |||
| # Contributing to lance-ray | |||
There was a problem hiding this comment.
not related with this PR
|
I have checked your branch, I believe the main reason that the code looks complicated was due to you used merge instead of rebase when creating the code. To make the diff clean, you can squash your incremental code into a commit, then based on the latest main, cherry pick that. In that way, you may make the diff clean |
|
Hi @chenghao-guo and @jackye1995 sir, I wanted to start by apologizing especially to @chenghao-guo sir for making those mistakes again and again in the previous PRs. I've created a new clean PR "feat: make pandas optional clean version #52" for the "Make pandas optional" feature (#35) following your feedback. This addresses all the previous concerns about messy diffs and unrelated files. Just wanted to share that this is my 3rd week working on this issue (Assigned by @jackye1995), and through all the challenges, I've actually learned so much about Git. Day by day, my Git commands are getting stronger and I'm gaining real experience with rebase, cherry-picking, and conflict resolution. I don't know if this new PR will be merged or rejected, but I wanted to thank you for your patience and guidance throughout this learning journey! 🙏 Please review the new clean PR when you have time. |
chenghao-guo
force-pushed
the
make-pandas-optional
branch
3 times, most recently
from
14716f9 to
a90e52c
Compare
…[data]) Co-authored-by: chenghao <landonguo@gmail.com>
chenghao-guo
force-pushed
the
make-pandas-optional
branch
from
2633540 to
69c1b9d
Compare
2 participants
This pull request addresses Issue #35 by making pandas an optional dependency in the project.
Updated the code to work without requiring pandas.
Added notes in documentation files explaining pandas is optional.
Ensured existing functionality remains unaffected when pandas is not installed.