Skip to content

Conversation

@5j9
Copy link
Contributor

@5j9 5j9 commented Mar 23, 2023

…windows

On Windows, zoneinfo relies on tzinfo and without it the tests fail with import error.

Also, rewrite pickle file loading.
Previously open was using a relative path, which means the path used to depend on the current working directory and could fail if tests were triggered from a wrong directory.
The new load_pickle function will handle that.

5j9 added 2 commits March 23, 2023 13:16
…windows

On Windows, zoneinfo relies on tzinfo and without it the tests fail with
import error.

Also, rewrite pickle file loading.
Previously `open` was using a relative path, which means the path
used to depend on the current working directory and could fail if
tests were triggered from a wrong directory.
The new load_pickle function will handle that.
import it as _, so that no name conflict occurs at line 750.
@slashmili slashmili requested a review from hramezani March 28, 2023 10:16
@slashmili
Copy link
Owner

@5j9 / @hramezani do you know how to fix the build in windows? https://ci.appveyor.com/project/slashmili/python-jalali

If we want to apply this changes, I'd rather to fix the appveyor as well.

@slashmili slashmili self-requested a review March 28, 2023 10:18
@5j9
Copy link
Contributor Author

5j9 commented Mar 28, 2023

@slashmili I just triggered an Appveyor build for this commit and it worked fine: https://ci.appveyor.com/project/5j9/python-jalali

I read somewhere that this particular error may occur if Appveyor's authorization to read your Github account has expired. You may need to revoke the Github access by going to https://ci.appveyor.com/authorizations and then reauthorize it.

@slashmili
Copy link
Owner

Thanks @5j9! It was indeed authorization issue!

Copy link
Owner

@slashmili slashmili left a comment

Choose a reason for hiding this comment

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

LGTM 👍🏼

@slashmili slashmili merged commit 8f79a04 into slashmili:main Mar 28, 2023
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.

2 participants