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

[unitaryHack] Add instructions to solve make docs error for Windows/3.8 users #691

Merged
merged 7 commits into from
May 27, 2021

Conversation

andre-a-alves
Copy link
Contributor

Description

Closes #689

As described in the issue, Windows users need to edit a file in their asyncio environment which affects non-Unix-like systems running Python 3.8 or higher.

Checklist

Check off the following once complete (or if not applicable) after opening the PR. The PR will be reviewed once this checklist is complete and all tests are passing.

If some items remain, you can mark this a draft pull request.

Tips

  • If the validation check fails:

    1. Run make check-style (from the root directory of the repository) and fix any flake8 errors.

    2. Run make format to format your code with the black autoformatter.

    For more information, check the Mitiq style guidelines.

  • Write "Fixes #XYZ" in the description if this PR fixes Issue #XYZ.

@codecov
Copy link

codecov bot commented May 22, 2021

Codecov Report

Merging #691 (fcd34d5) into master (74ec94e) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #691   +/-   ##
=======================================
  Coverage   96.96%   96.96%           
=======================================
  Files          42       42           
  Lines        2173     2173           
=======================================
  Hits         2107     2107           
  Misses         66       66           

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 74ec94e...fcd34d5. Read the comment docs.

@nathanshammah nathanshammah self-requested a review May 24, 2021 09:40
CONTRIBUTING.md Outdated
Comment on lines 50 to 52
To prevent errors when running `make docs` and `make doctest`, Windows developers using Pythong 3.8 will also need to edit \_\_init\_\_.py in their environment's asyncio directory.
This is due to asyncio changing their default event loop beginning in Python 3.8, and the default event loop will not support Unix-style APIs used by some dependencies.
1. Locate your environment directory (likely C:\Users\{username}\anaconda3\envs\\{your_env}), and open {env_dir}/Lib/asyncio/\_\_init\_\_.py.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
To prevent errors when running `make docs` and `make doctest`, Windows developers using Pythong 3.8 will also need to edit \_\_init\_\_.py in their environment's asyncio directory.
This is due to asyncio changing their default event loop beginning in Python 3.8, and the default event loop will not support Unix-style APIs used by some dependencies.
1. Locate your environment directory (likely C:\Users\{username}\anaconda3\envs\\{your_env}), and open {env_dir}/Lib/asyncio/\_\_init\_\_.py.
To prevent errors when running `make docs` and `make doctest`, Windows developers using Python 3.8 will also need to edit `\_\_init\_\_.py` in their environment's asyncio directory.
This is due to `asyncio` changing their default event loop beginning in Python 3.8, and the default event loop will not support Unix-style APIs used by some dependencies.
1. Locate your environment directory (likely `C:\Users\{username}\anaconda3\envs\\{your_env}`), and open `{env_dir}/Lib/asyncio/\_\_init\_\_.py`.

Copy link
Member

Choose a reason for hiding this comment

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

I also suggest linking something about asyncio.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Link added, and changes incorporated

Comment on lines +55 to +71
* Original Code

if sys.platform == 'win32': # pragma: no cover
from .windows_events import *
__all__ += windows_events.__all__
else:
from .unix_events import * # pragma: no cover
__all__ += unix_events.__all__

* Replacement Code

if sys.platform == 'win32': # pragma: no cover
from .windows_events import *
asyncio.set_event_loop_policy(asyncio.WindowsSelectorEventLoopPolicy())
__all__ += windows_events.__all__
else:
from .unix_events import * # pragma: no cover
Copy link
Member

Choose a reason for hiding this comment

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

This could be done the way we format code in markdown.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is formatted such that is appears as a nested code block instead of a full-width code block. But if you would like it full-width instead of nested, I can change it.
Screenshot 2021-05-24 160632

@rmlarose rmlarose requested a review from crazy4pi314 May 24, 2021 18:43
Copy link
Contributor

@rmlarose rmlarose left a comment

Choose a reason for hiding this comment

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

Thanks @andre-a-alves for the clear and detailed problem description / instructions. I pinged @crazy4pi314 who is on windows for review.

Did you find these instructions anywhere? Or was this a fix you came to after experimenting? I'm a bit worried about telling people they have to modify source files of asyncio in order to develop Mitiq - or at least Mitiq docs - on windows.

@andre-a-alves
Copy link
Contributor Author

I found the fix being used in a few different places. It was an overall problem with Jupyter that has been fixed as of 6.0.3, but one of our dependencies (probably myst-nb, but I'm not positive) does not seem to have overcome the issue, yet. I am assuming it comes from Jupyter, but it could potentially come from elsewhere.

Here is a GitHub link where Tornado implemented this fix, a StackExchange link that addresses this issue, and another StackExchange link that implements the same fix.

I am not a fan of system-wide changes, but since this is just within a Python environment, I figured it's not too bad, especially since it only affects developers but not users. I also think the issue will eventually go away, but there's no telling when that will happen.

@rmlarose
Copy link
Contributor

Gotcha, thanks for the response!! With this it seems like a reasonable (temp) fix to me.

Copy link
Member

@nathanshammah nathanshammah left a comment

Choose a reason for hiding this comment

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

Thanks @andre-a-alves, my bad! Looks good to me.

@nathanshammah nathanshammah merged commit c63e542 into unitaryfund:master May 27, 2021
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.

"make docs" raises error on Windows with Python 3.8
3 participants