arrow-py / arrow

🏹 Better dates & times for Python

https://arrow.readthedocs.io

Apache License 2.0

8.7k stars 673 forks source link

Add Sami locale #989

Closed cyriaka90 closed 3 years ago

cyriaka90 commented 3 years ago

Pull Request Checklist

Thank you for taking the time to improve Arrow! Before submitting your pull request, please check all appropriate boxes:

[x] 🧪 Added tests for changed code.
[x] 🛠️ All tests pass when run locally (run tox or make test to find out!).
[x] 🧹 All linting checks pass when run locally (run tox -e lint or make lint to find out!).
[ ] 📚 Updated documentation for changed code.
[x] ⏩ Code is up-to-date with the master branch.

Description of Changes

Adds Sami locale. :)

References for example: https://en.wikivoyage.org/wiki/Northern_S%C3%A1mi_phrasebook

codecov[bot] commented 3 years ago

Codecov Report

Merging #989 (1d1e6c7) into master (197bd64) will not change coverage. The diff coverage is 100.00%.

@@            Coverage Diff            @@
##            master      #989   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           10        10           
  Lines         2074      2083    +9     
  Branches       332       332           
=========================================
+ Hits          2074      2083    +9

Impacted Files	Coverage Δ
arrow/constants.py	`100.00% <ø> (ø)`
arrow/locales.py	`100.00% <100.00%> (ø)`

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 197bd64...1d1e6c7. Read the comment docs.

anishnya commented 3 years ago

@cyriaka90, I think this locale should support dehumanize. Do you mind adding the locale into the dehumanize test cases and in constants.py? Other than that, LGTM. @krisfremen any additional thoughts?

cyriaka90 commented 3 years ago

Of course, I can do that! :) I will update the PR accordingly soon.

cyriaka90 commented 3 years ago

I added the locale into the dehumanize test cases and in constants.py.

For the tests to succeed I added a blank space for future - I couldn't find a preposition/postposition for Sami here, so I assumed none is needed here, but I'm not sure what to do here to satisfy the tests more elegantly: https://github.com/arrow-py/arrow/blob/1d1e6c78b150981f828c601177d0424af0329850/arrow/locales.py#L4909 Any thoughts?

anishnya commented 3 years ago

couldn't find a preposition/postposition for Sam

That is the most elegant solution I can think of as well. The Basque locale had a similar issue as well when it comes to supporting dehumanize (we don't have a future proposition for Basque too). I think for now, I'll merge in this PR in and I'll create a separate issue and see what I can do on the dehumanize end of things for this.

As always, great work @cyriaka90! Also, if you have any thoughts on what should be added to the locale implementation guide, please feel free to add them on #983.

cyriaka90 commented 3 years ago

That is the most elegant solution I can think of as well. The Basque locale had a similar issue as well when it comes to supporting dehumanize (we don't have a future proposition for Basque too). I think for now, I'll merge in this PR in and I'll create a separate issue and see what I can do on the dehumanize end of things for this.

Alright, thanks! :)

As always, great work @cyriaka90! Also, if you have any thoughts on what should be added to the locale implementation guide, please feel free to add them on #983.

Thank you! I will do so!