Closed deependujha closed 5 days ago
Attention: Patch coverage is 90.76923%
with 6 lines
in your changes missing coverage. Please review.
Please upload report for BASE (
main@d5eff39
). Learn more about missing BASE report.
It needs to be tested to see if it works for S3, as it requires pro account to set up s3 connection with LightningAI studio.
But, if it works, then one improvement can be done:
If the incompatible (
different config
) datasets areappended
oroverwritten
, the current behavior is to do all the operations and then in the end, try to merge/ overwrite them.So, the better approach will be to check for compatibility initially.
Also, tests are passing in the lightning studio. I don't know why they failed here in mac and windows.
Before submitting
- [x] Was this discussed/agreed via a Github issue? (no need for typos and docs improvements) - [ ] Did you read the [contributor guideline](https://github.com/Lightning-AI/lit-data/blob/main/.github/CONTRIBUTING.md), Pull Request section? - [ ] Did you make sure to update the docs? - [x] Did you write any new necessary tests?What does this PR do?
Fixes #23
A test to understand this feature best is:
PR review
Anyone in the community is free to review the PR once the tests have passed. If we didn't discuss your PR in GitHub issues there's a high chance it will not be merged.
Did you have fun?
Make sure you had fun coding 🙃