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

Add Barbados holidays #1153

Closed
arkid15r opened this issue May 5, 2023 · 35 comments · Fixed by #1393
Closed

Add Barbados holidays #1153

arkid15r opened this issue May 5, 2023 · 35 comments · Fixed by #1393
Assignees

Comments

@arkid15r
Copy link
Collaborator

arkid15r commented May 5, 2023

Useful links:

@arjunanan6
Copy link
Contributor

Assign to me please, I'd like to help out. Possibly even with more.

@arkid15r
Copy link
Collaborator Author

arkid15r commented Jul 5, 2023

@arjunanan6 you got it.

Thank you!

@arjunanan6
Copy link
Contributor

@arkid15r Pretty much done here, but I can't commit.. I get a lot of errors relating to the pre-checks:

[INFO] Stashing unstaged files to /Users/arjun/.cache/pre-commit/patch1689880665-42063.
check python ast.........................................................Passed
check builtin type constructor use.......................................Passed
check yaml...........................................(no files to check)Skipped
fix end of files.........................................................Passed
fix python encoding pragma...............................................Passed
mixed line ending........................................................Passed
trim trailing whitespace.................................................Failed
- hook id: trailing-whitespace
- exit code: 1
- files were modified by this hook

Fixing tests/countries/test_barbados.py

black....................................................................Failed
- hook id: black
- files were modified by this hook

reformatted holidays/countries/barbados.py

All done! ✨ 🍰 ✨
1 file reformatted, 1 file left unchanged.

flake8...................................................................Failed
- hook id: flake8
- exit code: 1

holidays/countries/barbados.py:66:100: E501 line too long (110 > 99 characters)

isort....................................................................Failed
- hook id: isort
- files were modified by this hook

Fixing /Users/arjun/Documents/git/python-holidays/holidays/countries/barbados.py

mypy.....................................................................Passed
rst ``code`` is two backticks........................(no files to check)Skipped
rstcheck.............................................(no files to check)Skipped
tox-ini-fmt..........................................(no files to check)Skipped
setup-cfg-fmt........................................(no files to check)Skipped
[WARNING] Stashed changes conflicted with hook auto-fixes... Rolling back fixes...

Any idea what's happening here? I believe I set up everything according to the contributor's guide.

@arjunanan6
Copy link
Contributor

Issue is with: make l10n

@KJhellico
Copy link
Collaborator

@arjunanan6 try to run make pre-commit, this should fix these errors.

@arjunanan6
Copy link
Contributor

@KJhellico Thanks. make pre-commit goes through without a hitch, but when I push my changes it once again fails at "run tests":

run tests................................................................Failed
- hook id: tests
- exit code: 2

make l10n
make[1]: Entering directory 'C:/Users/arjun/Documents/git/python-holidays'
scripts/l10n/generate_po_files.py >/dev/null 2>&1
make[1]: *** [Makefile:27: l10n] Error 1
make[1]: Leaving directory 'C:/Users/arjun/Documents/git/python-holidays'
make: *** [Makefile:15: check] Error 2

@arkid15r
Copy link
Collaborator Author

The pre-commit runs make check (includes make l10n, make pre-commit, make doc, and make test) command via pre-commit/pre-push hooks.

It seems the problem is w/ scripts/l10n/generate_po_files.py. Its output is suppressed when running via make l10n
Could you run scripts/l10n/generate_po_files.py directly for (hopefully) more descriptive error message?

A link to your exported code branch could also help to troubleshoot it.

My guess is that .po files have some unresolved conflicts (or malformated in some other way).

@arjunanan6
Copy link
Contributor

arjunanan6 commented Jul 21, 2023

A link to your exported code branch could also help to troubleshoot it.

I can't commit as it stands, so can't even get the branch pushed. :(

I just ran scripts/l10n/generate_po_files.py and this is the output:

Traceback (most recent call last):
  File "C:\Users\arjun\Documents\git\python-holidays\scripts\l10n\generate_po_files.py", line 111, in <module>
    POGenerator.run()
  File "C:\Users\arjun\Documents\git\python-holidays\scripts\l10n\generate_po_files.py", line 107, in run
    POGenerator().process_countries()
  File "C:\Users\arjun\Documents\git\python-holidays\scripts\l10n\generate_po_files.py", line 73, in process_countries
    create_pot_file(
  File "C:\Users\arjun\AppData\Local\Programs\Python\Python310\lib\site-packages\click\core.py", line 1130, in __call__
    return self.main(*args, **kwargs)
  File "C:\Users\arjun\AppData\Local\Programs\Python\Python310\lib\site-packages\click\core.py", line 1055, in main
    rv = self.invoke(ctx)
  File "C:\Users\arjun\AppData\Local\Programs\Python\Python310\lib\site-packages\click\core.py", line 1404, in invoke
    return ctx.invoke(self.callback, **ctx.params)
  File "C:\Users\arjun\AppData\Local\Programs\Python\Python310\lib\site-packages\click\core.py", line 760, in invoke
    return __callback(*args, **kwargs)
  File "C:\Users\arjun\AppData\Local\Programs\Python\Python310\lib\site-packages\lingua\extract.py", line 426, in main
    for message in extractor(real_filename, extractor_options):
  File "C:\Users\arjun\AppData\Local\Programs\Python\Python310\lib\site-packages\lingua\extractors\python.py", line 415, in __call__
    return parser(token_stream, options, filename, lineno)
  File "C:\Users\arjun\AppData\Local\Programs\Python\Python310\lib\site-packages\lingua\extractors\python.py", line 187, in __call__
    for (token_type, token, location, _) in token_stream:
  File "C:\Users\arjun\AppData\Local\Programs\Python\Python310\lib\site-packages\lingua\extractors\python.py", line 162, in next
    token = self._transform(next(self.queue))
  File "C:\Users\arjun\AppData\Local\Programs\Python\Python310\lib\tokenize.py", line 452, in _tokenize
    line = readline()
  File "C:\Users\arjun\AppData\Local\Programs\Python\Python310\lib\site-packages\lingua\extractors\python.py", line 122, in __call__
    line = self.readline()
  File "C:\Users\arjun\AppData\Local\Programs\Python\Python310\lib\encodings\cp1252.py", line 23, in decode
    return codecs.charmap_decode(input,self.errors,decoding_table)[0]
UnicodeDecodeError: 'charmap' codec can't decode byte 0x81 in position 2954: character maps to <undefined>

@arjunanan6
Copy link
Contributor

I've tried this out on my win. machine by the way, on my mac it gives me all the errors from my first comment. I tried out the same files on win. where it reports just this issue with l10n.

@KJhellico
Copy link
Collaborator

KJhellico commented Jul 21, 2023

On Windows, localization process is currently problematic. If you want to try this on Win, to solve encoding problem, you need to set environment variable:

set PYTHONUTF8=1

(and better to delete holidays\locale\pot\*.* files, if any exist). So it's easier to try it on Mac, if you can.

on my mac it gives me all the errors from my first comment

It's pre-commit hook output. These errors were fixed by make pre-commit. Now try to run scripts/l10n/generate_po_files.py there, on Mac. What will it show?

@arjunanan6
Copy link
Contributor

I ran scripts/l10n/generate_po_files.py now on my mac and it outputs:

No changes found - not replacing holidays/locale/pot/AM.pot
No changes found - not replacing holidays/locale/pot/AR.pot
No changes found - not replacing holidays/locale/pot/AW.pot
No changes found - not replacing holidays/locale/pot/BA.pot
No translatable strings found, aborting

@arjunanan6
Copy link
Contributor

And this is the output of make pre-commit from the mac:

pre-commit run --all-files
check python ast.........................................................Passed
check builtin type constructor use.......................................Passed
check yaml...............................................................Passed
fix end of files.........................................................Passed
fix python encoding pragma...............................................Passed
mixed line ending........................................................Passed
trim trailing whitespace.................................................Passed
black....................................................................Passed
flake8...................................................................Passed
isort....................................................................Passed
mypy.....................................................................Failed
- hook id: mypy
- exit code: 1

scripts/generate_release_notes.py:82: error: Module has no attribute "BooleanOptionalAction"  [attr-defined]
Found 1 error in 1 file (checked 314 source files)

rst ``code`` is two backticks............................................Passed
rstcheck.................................................................Passed
tox-ini-fmt..............................................................Passed
setup-cfg-fmt............................................................Passed
make: *** [pre-commit] Error 1

@KJhellico
Copy link
Collaborator

I ran scripts/l10n/generate_po_files.py now on my mac and it outputs:

Try to delete holidays/locale/pot/*.pot files first.

scripts/generate_release_notes.py:82: error: Module has no attribute "BooleanOptionalAction" [attr-defined]

What's your Python version? This argparse option was added in 3.9. @arkid15r , looks like we have a bit of an inconsistency here 🙄

@arjunanan6
Copy link
Contributor

Deleted *.pot files as suggested, but I am not really seeing a difference. My python version is 3.8.5.

@arjunanan6
Copy link
Contributor

Do you think upgrading to 3.9 will help? I have no problems doing that.

@KJhellico
Copy link
Collaborator

Yes, try with 3.9 minimum.

@arkid15r
Copy link
Collaborator Author

scripts/generate_release_notes.py:82: error: Module has no attribute "BooleanOptionalAction" [attr-defined]
What's your Python version? This argparse option was added in 3.9. @arkid15r , looks like we have a bit of an inconsistency here 🙄

Yeah, I guess # type: ignore[attr-defined] should be added to L82. It's not a big deal as the script is for internal purposes, but you're right, we should address that.

@arjunanan6 either that or upgrade to python 3.9 should help.

@arjunanan6
Copy link
Contributor

So I am now using python 3.11.1 and have set up a new venv and ran make setup.

Following that, I ran make pre-commit which outputs:

check python ast.........................................................Passed
check builtin type constructor use.......................................Passed
check yaml...............................................................Passed
fix end of files.........................................................Passed
fix python encoding pragma...............................................Passed
mixed line ending........................................................Passed
trim trailing whitespace.................................................Passed
black....................................................................Passed
flake8...................................................................Passed
isort....................................................................Passed
mypy.....................................................................Passed
rst ``code`` is two backticks............................................Passed
rstcheck.................................................................Passed
tox-ini-fmt..............................................................Passed
setup-cfg-fmt............................................................Passed

However, if I were to commit something after that, it is back to the same situation as before:

% git commit -m "Add Barbados holidays"
[WARNING] Unstaged files detected.
[INFO] Stashing unstaged files to /Users/arjun/.cache/pre-commit/patch1690020648-79795.
check python ast.........................................................Passed
check builtin type constructor use.......................................Passed
check yaml...........................................(no files to check)Skipped
fix end of files.........................................................Passed
fix python encoding pragma...............................................Passed
mixed line ending........................................................Passed
trim trailing whitespace.................................................Failed
- hook id: trailing-whitespace
- exit code: 1
- files were modified by this hook

Fixing tests/countries/test_barbados.py

black....................................................................Failed
- hook id: black
- files were modified by this hook

reformatted holidays/countries/barbados.py

All done! ✨ 🍰 ✨
1 file reformatted, 1 file left unchanged.

flake8...................................................................Failed
- hook id: flake8
- exit code: 1

holidays/countries/barbados.py:66:100: E501 line too long (110 > 99 characters)

isort....................................................................Failed
- hook id: isort
- files were modified by this hook

Fixing /Users/arjun/Documents/git/python-holidays/holidays/countries/barbados.py

mypy.....................................................................Passed
rst ``code`` is two backticks........................(no files to check)Skipped
rstcheck.............................................(no files to check)Skipped
tox-ini-fmt..........................................(no files to check)Skipped
setup-cfg-fmt........................................(no files to check)Skipped
[WARNING] Stashed changes conflicted with hook auto-fixes... Rolling back fixes...

I'm at a loss :/

@KJhellico
Copy link
Collaborator

> [WARNING] Unstaged files detected.
> [INFO] Stashing unstaged files to /Users/arjun/.cache/pre-commit/patch1690020648-79795.

Maybe you have unstaged changes made by pre-commit? Try to look with git status and add them with git add if necessary.

@arjunanan6
Copy link
Contributor

Oh damn, that was it. Such a silly mistake! I can now run commit, however, when I push..:

check python ast.........................................................Passed
check builtin type constructor use.......................................Passed
check yaml...........................................(no files to check)Skipped
fix end of files.........................................................Passed
fix python encoding pragma...............................................Passed
mixed line ending........................................................Passed
trim trailing whitespace.................................................Passed
black....................................................................Passed
flake8...................................................................Passed
isort....................................................................Passed
mypy.....................................................................Passed
rst ``code`` is two backticks........................(no files to check)Skipped
rstcheck.............................................(no files to check)Skipped
tox-ini-fmt..........................................(no files to check)Skipped
setup-cfg-fmt........................................(no files to check)Skipped
run tests................................................................Failed
- hook id: tests
- exit code: 2

make l10n
scripts/l10n/generate_po_files.py >/dev/null 2>&1
make[1]: *** [l10n] Error 2
make: *** [check] Error 2

error: failed to push some refs to 'github.com:arjunanan6/python-holidays.git'

I can however run generate_po_files.py on its own without any errors.

@KJhellico
Copy link
Collaborator

> run tests................................................................Failed
> - hook id: tests
> - exit code: 2

Run make test, you should see errors.

@arjunanan6
Copy link
Contributor

I ran make test now, and indeed, I missed out on a doc change. That''s fixed, and make test runs without errors now. However, I am still getting an error when pushing:

check python ast.........................................................Passed
check builtin type constructor use.......................................Passed
check yaml...........................................(no files to check)Skipped
fix end of files.........................................................Passed
fix python encoding pragma...............................................Passed
mixed line ending........................................................Passed
trim trailing whitespace.................................................Passed
black....................................................................Passed
flake8...................................................................Passed
isort....................................................................Passed
mypy.....................................................................Passed
rst ``code`` is two backticks............................................Passed
rstcheck.................................................................Passed
tox-ini-fmt..........................................(no files to check)Skipped
setup-cfg-fmt........................................(no files to check)Skipped
run tests................................................................Failed
- hook id: tests
- exit code: 2

make l10n
scripts/l10n/generate_po_files.py >/dev/null 2>&1
make[1]: *** [l10n] Error 2
make: *** [check] Error 2

error: failed to push some refs to 'github.com:arjunanan6/python-holidays.git'

Truncated output from make test:

1852 passed in 32.50s

@KJhellico
Copy link
Collaborator

scripts/l10n/generate_po_files.py executed without errors? And scripts/l10n/generate_mo_files.py too?

@arjunanan6
Copy link
Contributor

Yes, both of those execute without any errors.

@arjunanan6
Copy link
Contributor

As I understand, the issue is with make l10n. I can run make pre-commit just fine. But if run make l10n, I get this:

scripts/l10n/generate_po_files.py >/dev/null 2>&1
make: *** [l10n] Error 2

And there's no problem executing scripts/l10n/generate_mo_files.py or scripts/l10n/generate_po_files.py.

@arkid15r
Copy link
Collaborator Author

Well, that doesn't make too much sense to me as make l10 is just an alias for running those 2 script consequently (see https://github.com/vacanza/python-holidays/blob/master/Makefile#L26).

Does make l10 command fail when you run it explicitly?

As this is just a local hook you should be able to suppress it w/ --no-verify flag and export your code. This would simplify the troubleshooting process a lot.

I hope this would work for you.

@arjunanan6
Copy link
Contributor

Does make l10 command fail when you run it explicitly?

It does, weirdly. I've looked at the Makefile and figured out what it does, but I can't seem to figure out why the two scripts execute individually, but not when run with make l10.

Anyway, I've pushed the changes with --no-verify. Thanks a lot for your support and patience!

@arkid15r
Copy link
Collaborator Author

That's a great progress! Thanks for getting it done, Arjun.
I'd think of it as a part of the process. I bet you already have or will learn something new during your work on this PR :)

@arjunanan6
Copy link
Contributor

Absolutely, and it was a great experience.
Sorry, I've made it against the master branch and not beta. Fixing that now and I will send in the new PR! :)

@arjunanan6 arjunanan6 mentioned this issue Jul 24, 2023
13 tasks
@arjunanan6
Copy link
Contributor

@arkid15r Sorry, I might need your help again, I've made a PR in the 'right' way, but it looks like it still wants to go to master rather than beta.

I've forked this repo and cloned my fork, switched to the beta branch and then made my own branch. Should do the trick normally. Or did I miss something?

@KJhellico
Copy link
Collaborator

You can select target branch when creating PR on Github

@KJhellico
Copy link
Collaborator

I changed the branch to beta :)

@arjunanan6
Copy link
Contributor

Nice, thank you! :)
I will keep that in mind for next time.

@arjunanan6
Copy link
Contributor

I suppose this one can be closed now @arkid15r.

@arkid15r
Copy link
Collaborator Author

That's right, I'll take care of it.
It hasn't been auto-closed after the merge as beta isn't the default branch (may be revisited soon).

Implemented by @arjunanan6 in #1393

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants