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

docs: Improved error message of encode_string #929

Merged
merged 1 commit into from
May 25, 2022

Conversation

frgfm
Copy link
Collaborator

@frgfm frgfm commented May 25, 2022

This PR improves the error message of encode_string when a character is not to be found in the vocab.

Closes #927

Any feedback is welcome!

@frgfm frgfm added topic: documentation Improvements or additions to documentation type: enhancement Improvement module: datasets Related to doctr.datasets labels May 25, 2022
@frgfm frgfm added this to the 0.5.2 milestone May 25, 2022
@frgfm frgfm self-assigned this May 25, 2022
@frgfm frgfm marked this pull request as ready for review May 25, 2022 19:08
@frgfm
Copy link
Collaborator Author

frgfm commented May 25, 2022

FYI, there is already a test checking that this case throws a ValueError (and I don't think we need to check the error message additionally 😅 )

@codecov
Copy link

codecov bot commented May 25, 2022

Codecov Report

Merging #929 (7be9cc2) into main (607aaaf) will decrease coverage by 0.01%.
The diff coverage is 50.00%.

@@            Coverage Diff             @@
##             main     #929      +/-   ##
==========================================
- Coverage   94.71%   94.70%   -0.02%     
==========================================
  Files         134      134              
  Lines        5488     5491       +3     
==========================================
+ Hits         5198     5200       +2     
- Misses        290      291       +1     
Flag Coverage Δ
unittests 94.70% <50.00%> (-0.02%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
doctr/datasets/utils.py 93.33% <50.00%> (-2.50%) ⬇️
doctr/transforms/functional/base.py 97.10% <0.00%> (+1.44%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 607aaaf...7be9cc2. Read the comment docs.

@felixdittrich92
Copy link
Contributor

@frgfm
Thanks LGTM :)
But next time i would recommend to give maybe up to 2 weeks for possible new contributors to solve some easy tasks 😅

@felixdittrich92 felixdittrich92 merged commit 0c8dd60 into mindee:main May 25, 2022
@frgfm frgfm deleted the encode-string branch May 25, 2022 22:11
@frgfm
Copy link
Collaborator Author

frgfm commented May 25, 2022

Yes I hesitated, but I figured no one had shown interest initially, but I'll keep that in mind next time 👍

Actually @fharper, I've seen a few OSS libraries keeping a few issues on the side for newcomers, and posting it on social media (and they got new contributors with that). That could be an idea 🤷

@felixdittrich92
Copy link
Contributor

@frgfm @fharper
That sounds like a good idea :)
Should we open a discussion to talk about some easy to solve tasks ? Or maybe better switch to Slack ?

@fharper
Copy link
Contributor

fharper commented May 26, 2022

Let's move the discussion on Slack, so we don't spam people here.

@frgfm frgfm modified the milestones: 0.5.2, 0.6.0 Jun 28, 2022
@felixdittrich92 felixdittrich92 modified the milestones: 0.5.2, 0.6.0 Sep 26, 2022
@felixdittrich92 felixdittrich92 mentioned this pull request Sep 26, 2022
85 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
module: datasets Related to doctr.datasets topic: documentation Improvements or additions to documentation type: enhancement Improvement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Fix encode_string function
3 participants