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.

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

Update map() and optimize() documentation #264

Merged

Conversation

senarvi
Copy link
Contributor

@senarvi senarvi commented Jul 24, 2024

Fixes documentation related to #260 .

Adds notes that the types and list sizes must not changed between samples, when calling optimize().

When looking at the code, I noticed that the documentation of the inputs seems to be outdated. AFAICS, inputs can be any sequence or a streaming dataloder, so I updated the type annotation and removed this from the docstring: "Each input should contain at least a valid filepath". Let me know if I missed something, or feel free to make changes in the PR.

My code editor also removed unnecessary whitespace. I guess that's good.

Copy link

codecov bot commented Jul 24, 2024

Codecov Report

Attention: Patch coverage is 60.00000% with 2 lines in your changes missing coverage. Please review.

Please upload report for BASE (main@27ba2ef). Learn more about missing BASE report.

Additional details and impacted files
@@          Coverage Diff          @@
##             main   #264   +/-   ##
=====================================
  Coverage        ?    78%           
=====================================
  Files           ?     34           
  Lines           ?   4874           
  Branches        ?      0           
=====================================
  Hits            ?   3785           
  Misses          ?   1089           
  Partials        ?      0           

@senarvi senarvi changed the title Updated map() and optimize() documentation Update map() and optimize() documentation Jul 24, 2024
@senarvi senarvi force-pushed the update-map-and-optimize-documentation branch 5 times, most recently from a3b5f6d to 4446d77 Compare July 24, 2024 08:46
@senarvi senarvi force-pushed the update-map-and-optimize-documentation branch from 4446d77 to 16350bf Compare July 24, 2024 08:49
@deependujha
Copy link
Collaborator

deependujha commented Jul 24, 2024

Hi @senarvi, thanks for opting to contribute to litdata.

Btw, Can you please convert your PR into draft until it's ready for review!

@senarvi
Copy link
Contributor Author

senarvi commented Jul 24, 2024

@deependujha how do I convert a PR into a draft?

@senarvi
Copy link
Contributor Author

senarvi commented Jul 24, 2024

Does anyone have an idea why the Windows and MacOS tests fail with "Numpy is not available"?

@deependujha
Copy link
Collaborator

@deependujha how do I convert a PR into a draft?

Screenshot from 2024-07-24 14-41-31

Just click on convert to Draft.


I'll try attempting the failing tests soon.

@senarvi
Copy link
Contributor Author

senarvi commented Jul 24, 2024

Thanks @deependujha , I haven't noticed that link! That will be useful in the future. From my behalf this PR is ready. I think anything in this PR is not responsible for breaking the tests, since this doesn't include functional changes. So better to leave this open, or what do you think?

@deependujha
Copy link
Collaborator

sure. I'm looking into it. Thanks for contributing.

@tchaton tchaton merged commit 25cb19f into Lightning-AI:main Jul 24, 2024
28 checks passed
Copy link
Collaborator

@deependujha deependujha left a comment

Choose a reason for hiding this comment

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

All failing tests were bcoz of numpy. But, recently support for numpy>=2 was added.

cc @tchaton

requirements/test.txt Show resolved Hide resolved
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants