ivy icon indicating copy to clipboard operation
ivy copied to clipboard

Published 20 hours ago • verified publisher icon Transpile-AI

Jax numpy log10

Open lumi232 opened this issue 2 years ago • 1 comments

I have added log10 to numpy jax frontend. log10 #5894

lumi232 avatar Oct 20 '22 15:10 lumi232

Hi! Thanks for the PR!

Before it can be merged, there are a few things that need to be fixed, but fortunately I don't think they'll be too bad to fix.

  1. GitHub Actions found some linting errors, these can be found in the checks higher up on this webpage, and can either be fixed manually or can be fixed by locally running flake8 and black. In the docs, there is a section on setting up pre-commit, which can automatically run those tools for you on each new commit you make, but it won't run them retroactively so you'll still have to fix the errors that are there now.
  2. The test for log10 is incomplete. If you look at the other tests in that file, you'll see that they have given and handle_cmd_line_args decorators, which must also be applied to the log10 test. It looks like the given decorator will be fairly simple, and you can validate that it works by running the test locally with pytest. If you'd like any help getting the tests running, or have any issues you can't work out how to fix, feel free to let me know and I'll see if I can help. This test will need to pass with a reasonable range of generated inputs to be merged into the master branch.
  3. The divide and remainder will need to have tests added in order to be merged. If you would like to remove those functions from this PR and add them with tests to a new PR in order to get this PR merged faster, that is fine with me. Once you have the test working for log10, you should have a good feel for what the tests for divide and remainder need to be, which will make this easier.

Overall however, I do think the PR looks good, just a few small issues before it can be merged.

Any questions or comments feel free to let me know!

JamieLine avatar Oct 20 '22 19:10 JamieLine

This PR has been labelled as stale because it has been inactive for more than 7 days. If you would like to continue working on this PR, then please add another comment or this PR will be closed in 7 days.

ivy-seed avatar Nov 06 '22 14:11 ivy-seed

This PR has been labelled as stale because it has been inactive for more than 7 days. If you would like to continue working on this PR, then please add another comment or this PR will be closed in 7 days.

ivy-seed avatar Nov 14 '22 06:11 ivy-seed

This PR has been closed because it has been marked as stale for more than 7 days with no activity.

ivy-seed avatar Nov 14 '22 07:11 ivy-seed