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 concept.py #73

Closed
wants to merge 1 commit into from
Closed

Conversation

Vortex-21
Copy link
Contributor

Description

Added validation to ensure strings are in proper format so they can be converted to floating point numbers .

Related Issue

#68

Changes Made

-Added validation with custom logic using pydantic field_validator .

Testing

Tested against cases :

  1. where string is not in proper float number format . (e.g : "abc", "12#", ".#", "", etc)
  2. where "value" is passed as float itself. (e.g : 12.0, .9, 8. ,etc.)

Checklist

  • I have read the contributing guidelines
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • [] I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • [] New and existing unit tests pass locally with my changes

Additional Notes

Added validation to ensure strings are in proper format so they can be converted to floating point numbers .
@Vortex-21
Copy link
Contributor Author

I have added the necessary validation procedure for the above issue.

@adamkells
Copy link
Contributor

Hey @Vortex-21, thanks for taking the time to contribute, we really appreciate it! This looks good, would you be able to add a test to validate that the code is working as expected? Also please make sure to post on the issue thread to let us know you're working on it so we can avoid any duplication of work.

@Vortex-21
Copy link
Contributor Author

Hey @Vortex-21, thanks for taking the time to contribute, we really appreciate it! This looks good, would you be able to add a test to validate that the code is working as expected? Also please make sure to post on the issue thread to let us know you're working on it so we can avoid any duplication of work.

Sure ! I will add the test file as well at the earliest .

@jenniferjiangkells
Copy link
Member

Closing this PR as this was completed in #77 - thanks for the contribution @Vortex-21!

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