num2words icon indicating copy to clipboard operation
num2words copied to clipboard

Spanish Implementation

Open Laifsyn opened this issue 1 year ago • 23 comments

Submitting as draft because 1-) I'm unclear how pull requests actually works 2-) I haven't properly made an Integration Test because I found it extremely un-ergonomic to constantly build the Num2Words settings after every .to_word() call

I also submitted a .devcontainer configuration because I found it extremely helpful to be able to test stuffs in a virtual environment specially so when I didn't have available a beefy-enough computer (I might've forgotten to add Rust Docs Viewer in the extensions though)

Laifsyn avatar Apr 07 '24 21:04 Laifsyn

Hey, sorry for the big delays in replying to the messages!

I'll take a look at your PR soon, thanks for the changes.

I'll be honest by saying I am not much interested in dev containers, do you think you can get them removed for now?

Also I've seen that you've done a lot of improvements regarding fmt, that's nice! I am wondering if it's too hard for you to split that in another PR? Usually one PR = one feature!

Thanks again :)

Ballasi avatar Apr 11 '24 01:04 Ballasi

oh yes. it's alright. As for the fmt it was something I ended up with after editing a template I copied and edited into my liking.

Laifsyn avatar Apr 11 '24 13:04 Laifsyn

just... give me a moment while I figure how to separate the stuffs out

Laifsyn avatar Apr 11 '24 13:04 Laifsyn

I'll assume the rustfmt thing will got to be in a separate PR right?

Laifsyn avatar Apr 11 '24 14:04 Laifsyn

that would be best! just move files that are not changed in this PR to another one. If there was rustfmt ran within files that were changes within this PR, I am completely fine with keeping it here (on top of that, that'll avoid merge conflicts), makes it easier for both you and me :)

Ballasi avatar Apr 11 '24 15:04 Ballasi

Sheesh. It makes me glad I didn't go crazy with changing stuffs. fortunately I found out about how to revert a file to a specific commit. Hope this is the way to go for such situation

Laifsyn avatar Apr 11 '24 17:04 Laifsyn

Should I also undo the added Derives to the enums?

Laifsyn avatar Apr 11 '24 17:04 Laifsyn

Alright, this should be about it. (except for mod.rs because I was too lazy to undo rustfmt on that. Shame on me) But everything should be the most minimal for the spanish implementation feature.

Rustfmt should be easily be doable in another pull request by just re-adding the rustfmt.toml file and running rustfmt against the project.

the undonde derives could be additionally done in another PR.

and optionally fix clippy warnings in another PR

Laifsyn avatar Apr 12 '24 16:04 Laifsyn

Sorry for making you go through all of this trouble! and thank you very much for taking the time to work on it, it's very kind :)

I'll see the PR more in depth this weekend! If you're not planning on redoing the derives or rustfmt, I'll take on that job once this is merged.

Ballasi avatar Apr 12 '24 19:04 Ballasi

I was thinking about figuring what rustfmt config was the one that onelined the chained methods and see if it was alright to disable it.

The reason is because if the method chaining gets one-lined, then the type-hint for each method return on the chain will not be hinted by rust-analyser.

but anyways, the chainings tends to be short more often than not, so the loss of the type-hint which can only be seen in an IDE isn't that relevant

Laifsyn avatar Apr 12 '24 20:04 Laifsyn

Hi, I was thinking about making a PR Benching with Criterion with another branch for benchmarks. Mainly I want to see what issues can arises when I expect merge conflicts( expecting so because I had to add some derives to do the benchmark)

The reason was to see how the spanish implementation fared because I did a fix to the unneeded 0 spam that the split_thousands() method created. (Ref: link)

Laifsyn avatar Apr 14 '24 17:04 Laifsyn

Alright. That should be it

Laifsyn avatar Apr 15 '24 05:04 Laifsyn

Oh, btw. I'm working on translating the currencies. Might submit a commit by the end of the day

Laifsyn avatar Apr 15 '24 15:04 Laifsyn

I have a question if you can get the answer

I'll be leaving it as the first meanwhile, but should it be like the second case?

assert_eq!(
    ordinal(124_001_091_000_000_000_001),
    "ciento veinticuatrotrillonésimos milnoventa y unobillonésimos primeros"
//  "ciento veinticuatrotrillonésimos mil noventa y unobillonésimos primeros"
);

Laifsyn avatar Apr 25 '24 15:04 Laifsyn

I couldn't redo the Cardinal implementation. Performance gains were dirt low. at about x1.02-x1.25 faster, and also the implementation looked a bit ugly. So I have no changes to the current implementation of Spanish.

Laifsyn avatar May 26 '24 03:05 Laifsyn

Hey guys! Native Spanish speaker here. Is there anything that you need help with? I'd love to use this crate in a project that I'm currently developing

PabloGmz96 avatar Jun 15 '24 22:06 PabloGmz96

Hey guys! Native Spanish speaker here. Is there anything that you need help with? I'd love to use this crate in a project that I'm currently developing

  • [ ] I would like opinions on whether to support a flavor for "Semantically Incorrect" milliards. according to RAE, 10E9(1,000,000,000) is called "Billón", whereas in English, 10E9 is called "Trillion". Choosing to support semantically Incorrect cardinals would mean that ~~spanish's "Billón" would be english's trillion~~ milliards like "billón" and "trillón" in spanish are just the literal translation of english's.

  • [ ] Maybe check currency, in case some currency got mistranslated.

  • [ ] ~~Support converting too big of numbers into nEm expression, i.e. 20.23E20 => veinte coma veintitrés por 10 a la 20~~

Priority close to 0:

  • Cleanup tests & Integration Tests: Because some test cases overlaps other tests. Also I personally think they could look cleaner with some work.

Otherwise, everything should be fine. Other than big numbers which can lose precision

Laifsyn avatar Jun 16 '24 02:06 Laifsyn

Hey @Laifsyn! I just review the "Semantically Incorrect" numbers that you mention and as far as I know almost the complete Latin-america region use those big numbers exactly in the same way as in English, so a direct translation of them should be precise enough. Later versions of this crate could include some other forms of Spanish cardinals numbers but knowing that they are rarely used this should be not a priority. But let me ask some friends in Colombia and Argentina (Countries that use particularly high number prices in the every day life) to confirm that.

In the other hand, let me test the USD and PESOS translation, those are the currencies that I'm familiar with.

And last but not least, I just want to say a big thank you to make the Spanish implementation possible!

PabloGmz96 avatar Jun 20 '24 08:06 PabloGmz96

Just a quick update... When using Spanish as language, the output shows the array that the code use to translate the numbers to words. image

PabloGmz96 avatar Jun 20 '24 09:06 PabloGmz96

Just a quick update... When using Spanish as language, the output shows the array that the code use to translate the numbers to words. image

Thanks for finding out! I might have forgotten to test the actual binary before I uploaded

Laifsyn avatar Jun 20 '24 13:06 Laifsyn

Any chance on getting this merged and pushed out?

bd-g avatar Jul 17 '25 01:07 bd-g

I just want to add that a year after PROD usage of this branch, I've found zero issues in Cardinal translation!

PabloGmz96 avatar Oct 01 '25 00:10 PabloGmz96

I just want to add that a year after PROD usage of this branch, I've found zero issues in Cardinal translation!

Wait, so it means there were issues in other places? 💀

Laifsyn avatar Nov 24 '25 02:11 Laifsyn