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.

Sign up for GitHub

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

Jump to bottom

[GSK-2280] Feature: Added Number-to-Words Transformation #1615

Merged
merged 26 commits into from
Jan 31, 2024
Merged

[GSK-2280] Feature: Added Number-to-Words Transformation #1615

merged 26 commits into from
Jan 31, 2024

Conversation

sagar118
Copy link
Contributor

Description

I have added the functionality to perform number-to-words transformation using the num2words library. I have added the unit test cases and dependency to the pdm.lock file.

Related Issue

Type of Change

  • 📚 Examples / docs / tutorials / dependencies update
  • 🔧 Bug fix (non-breaking change which fixes an issue)
  • 🥂 Improvement (non-breaking change which improves an existing feature)
  • 🚀 New feature (non-breaking change which adds functionality)
  • 💥 Breaking change (fix or feature that would cause existing functionality to change)
  • 🔐 Security fix

Checklist

  • I've read the CODE_OF_CONDUCT.md document.
  • I've read the CONTRIBUTING.md guide.
  • I've updated the code style using make codestyle.
  • I've written tests for all new methods and classes that I created.
  • I've written the docstring in Google format for all the methods and classes that I used.

Sorry, something went wrong.

@luca-martial luca-martial added the enhancement New feature or request label Nov 17, 2023
@luca-martial luca-martial linked an issue Nov 17, 2023 that may be closed by this pull request
Feature: add number-to-word transformation to transformation functions catalog #1567
Closed
@mattbit mattbit self-requested a review November 17, 2023 09:43
@luca-martial luca-martial changed the title [GSK-1567] Added Number-to-Words Transformation Feature: Added Number-to-Words Transformation Nov 17, 2023
mattbit
mattbit previously requested changes Nov 17, 2023
View reviewed changes
Copy link
Member

@mattbit mattbit left a comment

Choose a reason for hiding this comment

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

Thanks a lot for this very high-quality PR!

I added a few comments, but the main improvement over this would be to support multiple languages in the same dataset, without imposing a given language at the transformation level. We have metadata on the Dataset object that can be used to retrieve the detected language → you can check how this is done in the TextGenderTransformation.

Don’t hesitate to ping me if something is not clear, it’s a tricky part of the code. Also don’t hesitate to refactor other classes if you feel it’s necessary.

@mattbit
Copy link
Member

mattbit commented Dec 1, 2023

Hey @sagar118, unless you’ve already started looking into this, we can have someone from the team to take over and do the final changes :)

@sagar118
Copy link
Contributor Author

sagar118 commented Dec 1, 2023

Hey @mattbit, I apologize for the delay. I have worked on it a bit. If it's okay with you I can continue working on it and get back to you ASAP. Is there a timeline you're looking for to wrap up this issue?

@mattbit
Copy link
Member

mattbit commented Dec 1, 2023

Thanks, absolutely no problem. I think this can wait, should not be urgent, we are just reviewing pending PRs to avoid them getting stale. @kevinmessiaen can give you a more precise idea of the timeline.

In any case, if you think you don’t have time to complete, just let us know and we’ll have someone to finalize the minor edits needed before merging.

@kevinmessiaen
Merge remote-tracking branch 'origin/main' into GSK-1567-add-number-t…
1cd39ee 
…o-word-transformation

# Conflicts:
#	pdm.lock
#	pyproject.toml
@kevinmessiaen kevinmessiaen added the Lockfile Temporary label to update pdm.lock label Dec 14, 2023
@kevinmessiaen kevinmessiaen changed the title Feature: Added Number-to-Words Transformation [GSK-2280] Feature: Added Number-to-Words Transformation Dec 14, 2023
@sagar118 sagar118 requested a review from a team as a code owner December 19, 2023 08:42
@kevinmessiaen kevinmessiaen force-pushed the GSK-1567-add-number-to-word-transformation branch 4 times, most recently from 31a0c35 to e54ad2c Compare December 19, 2023 09:08
@kevinmessiaen kevinmessiaen force-pushed the GSK-1567-add-number-to-word-transformation branch from e54ad2c to c480246 Compare December 19, 2023 09:12
@mattbit mattbit added Lockfile Temporary label to update pdm.lock and removed Lockfile Temporary label to update pdm.lock labels Dec 19, 2023
@mattbit mattbit removed the Lockfile Temporary label to update pdm.lock label Dec 19, 2023
mattbit
mattbit reviewed Dec 22, 2023
View reviewed changes
Copy link
Member

@mattbit mattbit left a comment

Choose a reason for hiding this comment

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

Some extraneous code that maybe you should check @kevinmessiaen (problems with the automatic merge?)

@kevinmessiaen
Copy link
Member

Some extraneous code that maybe you should check @kevinmessiaen (problems with the automatic merge?)

Fixed this, it was a merge issue indeed

@kevinmessiaen kevinmessiaen requested a review from mattbit January 1, 2024 03:54
@kevinmessiaen kevinmessiaen dismissed mattbit’s stale review January 31, 2024 07:53

Changes has been implemented

@kevinmessiaen kevinmessiaen merged commit ec0f740 into Giskard-AI:main Jan 31, 2024
vjern added a commit that referenced this pull request Feb 1, 2024
@vjern
Squashed commit of the following:
d551411 
commit 71301e2
Author: Jocelyn Vernay <jocelyn@giskard.ai>
Date:   Thu Feb 1 17:11:58 2024 +0100

    Remove hardcoded failure

commit 919b144
Author: Jocelyn Vernay <jocelyn@giskard.ai>
Date:   Thu Feb 1 17:06:28 2024 +0100

    Update post-release error message

commit 24c897f
Author: Jocelyn Vernay <jocelyn@giskard.ai>
Date:   Thu Feb 1 17:00:43 2024 +0100

    Update post-release error message + Test it

commit c224f28
Author: Jocelyn Vernay <jocelyn@giskard.ai>
Date:   Thu Feb 1 16:37:27 2024 +0100

    Update messages

commit 333d315
Author: Jocelyn Vernay <jocelyn@giskard.ai>
Date:   Thu Feb 1 16:25:32 2024 +0100

    Fix commit link

commit e5d7867
Author: Jocelyn Vernay <jocelyn@giskard.ai>
Date:   Thu Feb 1 16:09:23 2024 +0100

    Update messages

commit 84c4d0a
Author: Jocelyn Vernay <jocelyn@giskard.ai>
Date:   Thu Feb 1 15:50:21 2024 +0100

    Update final slack notif

commit c0d5ec2
Author: Jocelyn Vernay <jocelyn@giskard.ai>
Date:   Thu Feb 1 15:47:12 2024 +0100

    Fix final slack notif

commit 1744a31
Author: Jocelyn Vernay <jocelyn@giskard.ai>
Date:   Thu Feb 1 15:33:28 2024 +0100

    Use vars.SLACK_CHANNEL_ID

commit 633471b
Author: Jocelyn Vernay <jocelyn@giskard.ai>
Date:   Thu Feb 1 15:30:18 2024 +0100

    Disable org member check

commit eb15bc7
Author: Jocelyn Vernay <jocelyn@giskard.ai>
Date:   Thu Feb 1 15:21:04 2024 +0100

    Fix slack_thread_id use

commit baa4dd3
Author: Jocelyn Vernay <jocelyn@giskard.ai>
Date:   Thu Feb 1 15:09:07 2024 +0100

    Fix slack hyperlinks

commit c832c35
Author: Jocelyn Vernay <jocelyn@giskard.ai>
Date:   Thu Feb 1 14:52:55 2024 +0100

    Break channel mentions

commit cc8c09e
Author: Jocelyn Vernay <jocelyn@giskard.ai>
Date:   Thu Feb 1 14:46:37 2024 +0100

    Fix `!cancelled()` step check

commit f6ac932
Author: Jocelyn Vernay <jocelyn@giskard.ai>
Date:   Thu Feb 1 14:43:23 2024 +0100

    Squash merge better-slack-notifications

    commit 99969d3
    Author: Jocelyn Vernay <jocelyn@giskard.ai>
    Date:   Thu Feb 1 14:16:54 2024 +0100

        Update slack steps names

    commit ffa2410
    Author: Jocelyn Vernay <jocelyn@giskard.ai>
    Date:   Thu Feb 1 13:03:35 2024 +0100

        Trim post-release

    commit e226ab2
    Author: vjern <jocelyn@giskard.ai>
    Date:   Thu Feb 1 11:46:05 2024 +0100

        Update release start message

    commit 4f75209
    Author: vjern <jocelyn@giskard.ai>
    Date:   Thu Feb 1 11:42:58 2024 +0100

        Fix `@channel` mention in create-release

    commit 3cf411e
    Author: BotReleaser <bot.releaser@users.noreply.github.com>
    Date:   Wed Jan 31 18:48:25 2024 +0100

        Add more slack notif ideas

    commit b53ce12
    Merge: 69901b5 c5da6cf
    Author: BotReleaser <bot.releaser@users.noreply.github.com>
    Date:   Wed Jan 31 16:59:06 2024 +0100

        Merge remote-tracking branch 'origin/feature/gsk-2574-improve-release-step' into feature/gsk-2574-better-slack-notifications

    commit c5da6cf
    Author: BotReleaser <bot.releaser@users.noreply.github.com>
    Date:   Wed Jan 31 16:47:20 2024 +0100

        Make slack channel id a repo Actions variable

    commit 5d3f2ce
    Author: BotReleaser <bot.releaser@users.noreply.github.com>
    Date:   Wed Jan 31 16:29:01 2024 +0100

        Fix `@channel` mention in post-release slack notif

    commit 69901b5
    Author: BotReleaser <bot.releaser@users.noreply.github.com>
    Date:   Wed Jan 31 16:00:08 2024 +0100

        Enable reploy_broadcast in post-release slack msg

    commit c494b43
    Author: BotReleaser <bot.releaser@users.noreply.github.com>
    Date:   Wed Jan 31 15:57:44 2024 +0100

        Expect and use slack thread id in post-release

    commit c5706de
    Author: BotReleaser <bot.releaser@users.noreply.github.com>
    Date:   Wed Jan 31 15:57:30 2024 +0100

        Add release author to release start message

    commit f07a3f4
    Merge: 402acf4 931f00e
    Author: BotReleaser <bot.releaser@users.noreply.github.com>
    Date:   Wed Jan 31 15:46:13 2024 +0100

        Merge branch 'feature/gsk-2574-improve-release-step' into feature/gsk-2574-better-slack-notifications

    commit 931f00e
    Author: BotReleaser <bot.releaser@users.noreply.github.com>
    Date:   Wed Jan 31 15:20:32 2024 +0100

        Fix @channel slack message

    commit b96f21f
    Author: BotReleaser <bot.releaser@users.noreply.github.com>
    Date:   Wed Jan 31 12:00:37 2024 +0100

        Add release pat token to post-release checkout

    commit 973f548
    Author: BotReleaser <bot.releaser@users.noreply.github.com>
    Date:   Wed Jan 31 11:48:40 2024 +0100

        Set wheel artifact retention period & err behavior

    commit f3ea326
    Author: BotReleaser <bot.releaser@users.noreply.github.com>
    Date:   Wed Jan 31 11:44:35 2024 +0100

        Name rc wheel artifact after input version number

    commit 402acf4
    Author: BotReleaser <bot.releaser@users.noreply.github.com>
    Date:   Wed Jan 31 10:44:15 2024 +0100

        Add release process start slack notification

    commit d942b8d
    Author: BotReleaser <bot.releaser@users.noreply.github.com>
    Date:   Wed Jan 31 10:41:49 2024 +0100

        Send success slack notif after publishing to pypi

    commit d1211af
    Merge: f4bd867 5ff5cd3
    Author: BotReleaser <bot.releaser@users.noreply.github.com>
    Date:   Tue Jan 30 18:05:49 2024 +0100

        Merge branch 'feature/gsk-2574-improve-release-step' of github.com:Giskard-AI/giskard into feature/gsk-2574-improve-release-step

    commit f4bd867
    Author: BotReleaser <bot.releaser@users.noreply.github.com>
    Date:   Tue Jan 30 18:05:06 2024 +0100

        Reset build-python

    commit 5ff5cd3
    Merge: 15386e3 1b724c6
    Author: Jocelyn Vernay <59055698+vjern@users.noreply.github.com>
    Date:   Tue Jan 30 14:38:34 2024 +0100

        Merge branch 'main' into feature/gsk-2574-improve-release-step

    commit 15386e3
    Author: BotReleaser <bot.releaser@users.noreply.github.com>
    Date:   Tue Jan 30 14:33:00 2024 +0100

        Restore step ifs and remove dev comments

    commit deb70a8
    Author: BotReleaser <bot.releaser@users.noreply.github.com>
    Date:   Tue Jan 30 14:26:45 2024 +0100

        Restore other diff spans unrelated to feature

    commit 439e7df
    Author: BotReleaser <bot.releaser@users.noreply.github.com>
    Date:   Tue Jan 30 14:24:05 2024 +0100

        Restore org member check steps

    commit 1867aae
    Author: BotReleaser <bot.releaser@users.noreply.github.com>
    Date:   Tue Jan 30 14:23:33 2024 +0100

        Remove  do-release

    commit b86a9e4
    Author: BotReleaser <bot.releaser@users.noreply.github.com>
    Date:   Tue Jan 30 14:22:00 2024 +0100

        Reset README

commit 29402e8
Author: vjern <jocelyn@giskard.ai>
Date:   Thu Feb 1 11:27:03 2024 +0100

    Send post-release end notif after pypi publishing

commit 43cc3e4
Author: BotReleaser <bot.releaser@users.noreply.github.com>
Date:   Wed Jan 31 17:27:46 2024 +0100

    Fix version name in post-release slack notif

commit 929d16f
Author: BotReleaser <bot.releaser@users.noreply.github.com>
Date:   Wed Jan 31 16:47:20 2024 +0100

    Make slack channel id a repo Actions variable

commit 5843d1e
Author: BotReleaser <bot.releaser@users.noreply.github.com>
Date:   Wed Jan 31 16:29:01 2024 +0100

    Fix `@channel` mention in post-release slack notif

commit 2b364fd
Author: BotReleaser <bot.releaser@users.noreply.github.com>
Date:   Wed Jan 31 11:44:35 2024 +0100

    Name rc wheel artifact after input version number

commit 6f35afe
Author: BotReleaser <bot.releaser@users.noreply.github.com>
Date:   Wed Jan 31 15:20:32 2024 +0100

    Fix @channel slack message

commit 7befb4e
Author: BotReleaser <bot.releaser@users.noreply.github.com>
Date:   Wed Jan 31 12:00:37 2024 +0100

    Add release pat token to post-release checkout

commit 1305e00
Author: BotReleaser <bot.releaser@users.noreply.github.com>
Date:   Wed Jan 31 11:48:40 2024 +0100

    Set wheel artifact retention period & err behavior

commit 39685f6
Merge: 63c8478 ec0f740
Author: Jocelyn Vernay <59055698+vjern@users.noreply.github.com>
Date:   Wed Jan 31 10:01:46 2024 +0100

    Merge branch 'Giskard-AI:main' into main

commit ec0f740
Merge: af8dcbd 1a52ffb
Author: Kevin Messiaen <114553769+kevinmessiaen@users.noreply.github.com>
Date:   Wed Jan 31 14:53:45 2024 +0700

    Merge pull request #1615 from sagar118/GSK-1567-add-number-to-word-transformation

    [GSK-2280] Feature: Added Number-to-Words Transformation

commit 1a52ffb
Author: Kevin Messiaen <kevin.messiaen@icloud.com>
Date:   Wed Jan 31 11:59:29 2024 +0700

    Limited scipy version to prevent nan issue with test data drift

commit 300b297
Merge: 27a100b af8dcbd
Author: Kevin Messiaen <114553769+kevinmessiaen@users.noreply.github.com>
Date:   Wed Jan 31 11:41:20 2024 +0700

    Merge branch 'main' into GSK-1567-add-number-to-word-transformation

commit af8dcbd
Merge: 1b724c6 bf57a24
Author: Matteo <matteo@giskard.ai>
Date:   Tue Jan 30 19:54:30 2024 +0100

    Merge pull request #1677 from Giskard-AI/task/avid-docs-gsk-2195

    Add documentation for AVID integration

commit bf57a24
Merge: 9f8c26f 1b724c6
Author: Matteo <matteo@giskard.ai>
Date:   Tue Jan 30 18:57:44 2024 +0100

    Merge branch 'main' into task/avid-docs-gsk-2195

commit 9f8c26f
Author: Matteo Dora <matteo@giskard.ai>
Date:   Tue Jan 30 18:57:03 2024 +0100

    Fixes after review

commit 27a100b
Merge: 6af109c 1b724c6
Author: Kevin Messiaen <114553769+kevinmessiaen@users.noreply.github.com>
Date:   Tue Jan 30 09:22:50 2024 +0700

    Merge branch 'main' into GSK-1567-add-number-to-word-transformation

commit beba75e
Merge: ed43133 da01798
Author: Hartorn <bazire@giskard.ai>
Date:   Mon Jan 29 14:05:41 2024 +0100

    Merge branch 'main' into task/avid-docs-gsk-2195

commit 6af109c
Author: Kevin Messiaen <kevin.messiaen@icloud.com>
Date:   Mon Jan 29 11:43:10 2024 +0700

    Updated PDM lockfile

commit 345c81f
Merge: 607122d 6df7e1a
Author: Kevin Messiaen <114553769+kevinmessiaen@users.noreply.github.com>
Date:   Mon Jan 29 11:05:06 2024 +0700

    Merge branch 'main' into GSK-1567-add-number-to-word-transformation

commit ed43133
Merge: 0e2fbdd 6df7e1a
Author: Matteo <matteo@giskard.ai>
Date:   Fri Jan 26 15:59:15 2024 +0100

    Merge branch 'main' into task/avid-docs-gsk-2195

commit 607122d
Merge: 19e7996 782f295
Author: Kevin Messiaen <kevin.messiaen@icloud.com>
Date:   Fri Jan 26 11:16:19 2024 +0700

    Merge branch 'main' into GSK-1567-add-number-to-word-transformation

commit 0e2fbdd
Merge: 6fab58f c34349d
Author: Andrey Avtomonov <andreybavt@gmail.com>
Date:   Thu Jan 4 11:17:31 2024 +0100

    Merge branch 'main' into task/avid-docs-gsk-2195

commit 19e7996
Merge: 2c76855 c34349d
Author: Andrey Avtomonov <andreybavt@gmail.com>
Date:   Thu Jan 4 11:16:42 2024 +0100

    Merge branch 'main' into GSK-1567-add-number-to-word-transformation

commit 2c76855
Merge: 1379407 bd478ef
Author: Andrey Avtomonov <andreybavt@gmail.com>
Date:   Tue Jan 2 21:23:23 2024 +0100

    Merge branch 'main' into GSK-1567-add-number-to-word-transformation

commit 6fab58f
Merge: 5df80ef bd478ef
Author: Andrey Avtomonov <andreybavt@gmail.com>
Date:   Tue Jan 2 21:23:05 2024 +0100

    Merge branch 'main' into task/avid-docs-gsk-2195

commit 1379407
Author: Kevin Messiaen <kevin.messiaen@icloud.com>
Date:   Mon Jan 1 10:54:36 2024 +0700

    Removed useless constructor

commit 866d8be
Author: Kevin Messiaen <kevin.messiaen@icloud.com>
Date:   Mon Jan 1 10:50:51 2024 +0700

    Wrong modifications

commit f3c71ef
Merge: 7131491 7cf84d3
Author: Kevin Messiaen <114553769+kevinmessiaen@users.noreply.github.com>
Date:   Mon Jan 1 10:42:33 2024 +0700

    Merge branch 'main' into GSK-1567-add-number-to-word-transformation

commit 7131491
Merge: c2db118 858c3c1
Author: Matteo <matteo@giskard.ai>
Date:   Fri Dec 22 15:54:39 2023 +0100

    Merge branch 'main' into GSK-1567-add-number-to-word-transformation

commit c2db118
Merge: f9dc4ca 908a707
Author: Kevin Messiaen <114553769+kevinmessiaen@users.noreply.github.com>
Date:   Fri Dec 22 09:24:15 2023 +0700

    Merge branch 'main' into GSK-1567-add-number-to-word-transformation

commit f9dc4ca
Merge: bfb6c98 0f7fa52
Author: Kevin Messiaen <114553769+kevinmessiaen@users.noreply.github.com>
Date:   Thu Dec 21 15:23:24 2023 +0700

    Merge branch 'main' into GSK-1567-add-number-to-word-transformation

commit bfb6c98
Author: Kevin Messiaen <kevin.messiaen@icloud.com>
Date:   Thu Dec 21 13:59:11 2023 +0700

    Fixed tests

commit 4691156
Merge: 9d9976f 597c8cc
Author: Kevin Messiaen <114553769+kevinmessiaen@users.noreply.github.com>
Date:   Thu Dec 21 12:14:04 2023 +0700

    Merge branch 'main' into GSK-1567-add-number-to-word-transformation

commit 9d9976f
Author: Hartorn <hartorn.github@gmail.com>
Date:   Tue Dec 19 14:32:28 2023 +0100

    Fixing lockfile and dependencies issues

commit 7884690
Merge: c480246 c67e9b4
Author: Matteo <matteo@giskard.ai>
Date:   Tue Dec 19 12:14:24 2023 +0100

    Merge branch 'main' into GSK-1567-add-number-to-word-transformation

commit c480246
Merge: f153538 ae77016
Author: Kevin Messiaen <114553769+kevinmessiaen@users.noreply.github.com>
Date:   Tue Dec 19 15:18:07 2023 +0700

    Merge branch 'main' into GSK-1567-add-number-to-word-transformation

commit f153538
Merge: 2208b5e b63c3d1
Author: Kevin Messiaen <114553769+kevinmessiaen@users.noreply.github.com>
Date:   Mon Dec 18 05:11:35 2023 +0100

    Merge branch 'main' into GSK-1567-add-number-to-word-transformation

commit 2208b5e
Merge: 0cb8fec ad84608
Author: Matteo <matteo@giskard.ai>
Date:   Thu Dec 14 16:21:40 2023 +0100

    Merge branch 'main' into GSK-1567-add-number-to-word-transformation

commit 5df80ef
Merge: fe23ad4 2a6dfaf
Author: Hartorn <bazire@giskard.ai>
Date:   Thu Dec 14 11:39:23 2023 +0100

    Merge branch 'main' into task/avid-docs-gsk-2195

commit fe23ad4
Merge: c0de0c0 cc4d071
Author: Hartorn <bazire@giskard.ai>
Date:   Thu Dec 14 10:22:30 2023 +0100

    Merge branch 'main' into task/avid-docs-gsk-2195

commit 0cb8fec
Author: Kevin Messiaen <114553769+kevinmessiaen@users.noreply.github.com>
Date:   Thu Dec 14 08:42:36 2023 +0100

    Update tests/scan/test_text_transformations.py

    Co-authored-by: Matteo <me@mattbit.com>

commit 64a1f66
Author: Kevin Messiaen <kevin.messiaen@icloud.com>
Date:   Thu Dec 14 08:40:00 2023 +0100

    Updated TextNumberToWordTransformation to use row by row language instead of dataset language

commit 1cd39ee
Merge: bae694a cc4d071
Author: Kevin Messiaen <kevin.messiaen@icloud.com>
Date:   Thu Dec 14 08:28:19 2023 +0100

    Merge remote-tracking branch 'origin/main' into GSK-1567-add-number-to-word-transformation

commit c0de0c0
Author: Matteo Dora <matteo@giskard.ai>
Date:   Tue Dec 12 13:47:46 2023 +0100

    Add documentation for AVID integration

commit bae694a
Merge: 47d1a73 801ef04
Author: Matteo <matteo@giskard.ai>
Date:   Fri Nov 17 10:43:58 2023 +0100

    Merge branch 'main' into GSK-1567-add-number-to-word-transformation

commit 47d1a73
Author: Sagar Thacker <sgrhacker18@gmail.com>
Date:   Fri Nov 17 01:36:24 2023 -0500

    Added Numbers to Words Transformation
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kevinmessiaen kevinmessiaen

@mattbit mattbit

Assignees

@mattbit mattbit

Labels
enhancement New feature or request safe for build
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Feature: add number-to-word transformation to transformation functions catalog
6 participants
@sagar118 @mattbit @kevinmessiaen @luca-martial @Hartorn @andreybavt