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

refactor: Removed Optional typehint when the default value of an argument is non-None value #28034

Merged
merged 1 commit into from
Jan 25, 2024

Conversation

Sai-Suraj-27
Copy link
Contributor

PR Description

At few places in the codebase, when the default value of an argument is not None, still the Optional typehint is being used. It should be removed as recommended by mypy and for better clarity in general.
https://github.com/unifyai/ivy/blob/82ccaebd9743a6d7d0d47309fc0db3b43c315cbe/ivy/functional/backends/jax/experimental/losses.py#L33-L34

Related Issue

Closes #28033

Checklist

  • Did you add a function?
  • Did you add the tests?
  • Did you run your tests and are your tests passing?
  • Did pre-commit not fail on any check?
  • Did you follow the steps we provided?

Socials

Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

PR Compliance Checks Passed!

@Sai-Suraj-27 Sai-Suraj-27 changed the title Refactor: Removed Optional typehint when the default value of an argument is non-None value refactor: Removed Optional typehint when the default value of an argument is non-None value Jan 24, 2024
@Sai-Suraj-27 Sai-Suraj-27 changed the title refactor: Removed Optional typehint when the default value of an argument is non-None value refactor: Removed Optional typehint when the default value of an argument is non-None value Jan 24, 2024
@ivy-leaves ivy-leaves added Paddle Paddle Backend Ivy Functional API Ivy API Experimental Run CI for testing API experimental/New feature or request labels Jan 24, 2024
@vedpatwardhan
Copy link
Contributor

lgtm! Feel free to merge the PR @joaozenobio, thanks @Sai-Suraj-27 😄

@joaozenobio
Copy link
Contributor

Sure thing! Nice work @Sai-Suraj-27 😁

@joaozenobio joaozenobio merged commit e0d6493 into Transpile-AI:main Jan 25, 2024
520 of 557 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Ivy API Experimental Run CI for testing API experimental/New feature or request Ivy Functional API Paddle Paddle Backend
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Remove Optional typehint when the argument has non-None default value
4 participants