search

arrow-py / arrow

🏹 Better dates & times for Python
https://arrow.readthedocs.io
Apache License 2.0
8.7k stars 673 forks source link

Add Armenian locale #1096

Closed ElahehAx closed 2 years ago

ElahehAx commented 2 years ago

Pull Request Checklist

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

If you have any questions about your code changes or any of the points above, please submit your questions along with the pull request and we will try our best to help!

Description of Changes

Add Armenian locale

codecov[bot] commented 2 years ago

Codecov Report

Merging #1096 (e0ae368) into master (7b5c1aa) will not change coverage. The diff coverage is 100.00%.

@@            Coverage Diff            @@
##            master     #1096   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           10        10           
  Lines         2314      2325   +11     
  Branches       448       449    +1     
=========================================
+ Hits          2314      2325   +11
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 7b5c1aa...e0ae368. Read the comment docs.

anishnya commented 2 years ago

Hey @ElahehAx, the PR looks good to me. I'm not too sure why CodeCov says there's reduced coverage. I'll have to investigate further. @jadchaar @krisfremen @systemcatch any thoughts here? I don't see anything that would cause a coverage decrease.

systemcatch commented 2 years ago

Are we missing tests for the merdians? We dropped the coverage requirement below 100% recently but can't remember the exact figure.

anishnya commented 2 years ago

Are we missing tests for the merdians? We dropped the coverage requirement below 100% recently but can't remember the exact figure.

Looking at the CodeCov coverage, it looks like it says there's a gap in the humanize coverage? To me this doesn't make sense. Will need to investigate further.

ElahehAx commented 2 years ago

Hi. Thanks for your comments. Because of systemcatch's comment, I added some tests for meridians. I also deleted some duplicate tests.

ElahehAx commented 2 years ago

Hi, I saw that these tests failed, however the local test on my machine was successful,would you suggest anything I can do to resolve this issue?

anishnya commented 2 years ago

Hi, I saw that these tests failed, however the local test on my machine was successful,would you suggest anything I can do to resolve this issue?

So it looks like re-running the tests solved that issue. The coverage issue appears to be the fact CodeCov thinks coverage has been reduced on a specific branch of humanize. I'll investigate this later today, but I suspect it might be something to do with the way Dehumanize uses Humanize in testing that's causing this.

systemcatch commented 2 years ago

Hey @ElahehAx do you want to fix the conflicts or shall we do it? Our CI issue is hopefully fixed now. :crossed_fingers:

ElahehAx commented 2 years ago

hey @systemcatch of course. Please feel free to do it

ElahehAx commented 2 years ago

Hey @systemcatch and @anishnya, I updated the PR (conflicts and syntax error solved), but now "Lint docs" (https://github.com/arrow-py/arrow/runs/6741399878?check_suite_focus=true) has an error. Do you know please why it's failing?"

krisfremen commented 2 years ago

hey @ElahehAx it seems to be an issue with the docs generation.

It doesn't seem to be anything from your code though. Will take a look shortly!

krisfremen commented 2 years ago

hey @ElahehAx, seems like the culprit was sphinx, got a PR ready to fix that, as soon as it's in, I'll rerun the checks and merge this in.