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

Improve docs for some functions in the Series submodule #101

Merged
merged 6 commits into from
Jan 24, 2022
Merged

Improve docs for some functions in the Series submodule #101

merged 6 commits into from
Jan 24, 2022

Conversation

kimjoaoun
Copy link
Contributor

This pull request adds examples for the following functions Series.(first, add, last, multiply, subtract, distinct).

@cigrainger
Copy link
Member

cigrainger commented Jan 24, 2022

Thanks for this! Looks great. There is at least one failing test that looks like a spacing issue.

@kimjoaoun
Copy link
Contributor Author

kimjoaoun commented Jan 24, 2022

@cigrainger it is not a spacing issue, it is because Explorer.Series.distinct() do not maintain the order. Each execution returns a different result so the test always fails.

I removed the iex> calls from the distinct docs and now the tests should pass. I think the fast solution to this problem is enforcing a seed as we can see here, an optimal one would be making Series.distinct() maintain the order.

EDIT: I can also omit the results so we wouldn't have an assertion. What do you think?

@cigrainger
Copy link
Member

Ah -- this will be to do with the way Polars implements the distinct call, so seeding the Elixir side won't help. For now, would you mind omitting the results and opening an issue? Thanks!

@kimjoaoun
Copy link
Contributor Author

@cigrainger done. Now I'll open the issue :)

@cigrainger cigrainger merged commit 3536b2e into elixir-explorer:main Jan 24, 2022
@kimjoaoun kimjoaoun deleted the improve_docs branch January 24, 2022 22:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants