Skip to content

fixed a bunch of stuff#152

Merged
remsky merged 9 commits intoremsky:masterfrom
fireblade2534:fixedstuff
Feb 13, 2025
Merged

fixed a bunch of stuff#152
remsky merged 9 commits intoremsky:masterfrom
fireblade2534:fixedstuff

Conversation

@fireblade2534
Copy link
Collaborator

Made the api use the normalizer, fixed the docker compose not starting on windows because of line endings, fixed the wrong version of espeak, added better normilzation, improved the sentence splitting, fixed some formatting

…dded better normilzation, improved the sentence splitting, fixed some formatting
COPY --chown=appuser:appuser web ./web
COPY --chown=appuser:appuser docker/scripts/ ./
RUN chmod +x ./entrypoint.sh
RUN sed -i 's/\r$//' ./entrypoint.sh
Copy link

@fedot fedot Feb 11, 2025

Choose a reason for hiding this comment

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

Comment edited by @remsky to keep it productive and focused on the code

This issue happens due to your Git config on Windows adding Windows line endings to checked-out files. Setting core.autocrlf to input prevents this, eliminating the need for a workaround.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I didn’t know that thank you for informing me I’ll look into it and remove the workaround.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It works though @remsky u might wanna add a .gitatributes (I think that's what it called) so everyone has that auto set

# Install dependencies with GPU extras (using cache mounts)
RUN --mount=type=cache,target=/root/.cache/uv \
uv venv && \
uv venv --python 3.11 && \
Copy link

Choose a reason for hiding this comment

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

Why would you want to set it to a specific version if it's not a hard requirement?

Copy link
Owner

@remsky remsky Feb 11, 2025

Choose a reason for hiding this comment

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

Believe its part of investigating the best solution to the espeak dependency bugs, which has involved a few version bumps and comparisons on the image etc to narrow in on.

Appreciate you taking the time to review @fedot, open source is a team effort, and every bit helps that encourages to that end.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Why would you want to set it to a specific version if it's not a hard requirement?

Mostly because it was built and tested with Python 3.10 (I changed it in the latest version so its the correct version). The idea is that even if a package doesn't have a version for the latest version of python it will still be set to python 3.10 inside the container

"Ω":"ohm", "kΩ":"kiloohm", "mΩ":"megaohm", # Resistance (Ohm)
"f":"farad", "µf":"microfarad", "nf":"nanofarad", "pf":"picofarad", # Capacitance
"b":"byte", "kb":"kilobyte", "mb":"megabyte", "gb":"gigabyte", "tb":"terabyte", "pb":"petabyte", # Data size
"kbps":"kilobyte per second","mbps":"megabyte per second","gbps":"gigabyte per second",
Copy link

@fedot fedot Feb 11, 2025

Choose a reason for hiding this comment

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

Comment edited by @remsky to retain constructive elements

  • By convention: kb, mb, gb, etc., typically refer to bits, not bytes.
  • Similarly: kbps (or kb/s) means kilobits per second, while kBps (or kB/s) refers to kilobytes per second

The capitalization of the B shows the distinction

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I do agree the main issue I faced though is that some are capitalization independent while others are not but I do have some ideas so I will also look into that too. Thanks for the feedback.

Repository owner deleted a comment from fedot Feb 11, 2025
This was referenced Feb 11, 2025
@fireblade2534 fireblade2534 marked this pull request as draft February 11, 2025 16:00
@fedot
Copy link

fedot commented Feb 11, 2025

Folks, thank you both for the time and effort! 👍❤️
Appreciate the edits, didn't meant to offense anyone, sorry if my comments were a bit off, point taken!

@fireblade2534 fireblade2534 marked this pull request as ready for review February 12, 2025 02:35
@remsky remsky merged commit 728e18b into remsky:master Feb 13, 2025
1 check passed
ddillon183 pushed a commit to ddillon183/kokoro-fastapi-real that referenced this pull request Jun 12, 2025
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