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

Updated user guide introduction for nmigen. #24

Closed
nmigen-issue-migration opened this issue Jan 6, 2019 · 8 comments
Closed

Updated user guide introduction for nmigen. #24

nmigen-issue-migration opened this issue Jan 6, 2019 · 8 comments

Comments

@nmigen-issue-migration
Copy link

@nmigen-issue-migration nmigen-issue-migration commented Jan 6, 2019

Issue by sam-falvo
Sunday Jan 06, 2019 at 20:02 GMT
Originally opened as m-labs/nmigen#22


The goal is to work through each section, one by one, receiving documentation reviews along the way. The introduction is a good first step. Feedback appreciated!


sam-falvo included the following code: https://github.com/m-labs/nmigen/pull/22/commits

@nmigen-issue-migration
Copy link
Author

@nmigen-issue-migration nmigen-issue-migration commented Jan 6, 2019

Comment by codecov[bot]
Sunday Jan 06, 2019 at 20:11 GMT


Codecov Report

Merging #22 into master will increase coverage by 3.57%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #22      +/-   ##
==========================================
+ Coverage    82.3%   85.87%   +3.57%     
==========================================
  Files          34       28       -6     
  Lines        5565     4063    -1502     
  Branches     1190      818     -372     
==========================================
- Hits         4580     3489    -1091     
+ Misses        852      480     -372     
+ Partials      133       94      -39
Impacted Files Coverage Δ
nmigen/back/verilog.py 61.9% <0%> (-5.67%) ⬇️
nmigen/lib/io.py 80% <0%> (-2.36%) ⬇️
nmigen/compat/fhdl/module.py 71.15% <0%> (-1.82%) ⬇️
nmigen/compat/genlib/fsm.py 56.11% <0%> (-1.64%) ⬇️
nmigen/compat/fhdl/verilog.py 69.23% <0%> (-1.36%) ⬇️
nmigen/hdl/ast.py 86.19% <0%> (-0.64%) ⬇️
nmigen/compat/fhdl/specials.py 29.76% <0%> (-0.58%) ⬇️
nmigen/hdl/dsl.py 99.28% <0%> (-0.42%) ⬇️
nmigen/tracer.py 94.44% <0%> (-0.16%) ⬇️
nmigen/hdl/mem.py 98.05% <0%> (-0.04%) ⬇️
... and 21 more

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 51269ad...1fedc1d. Read the comment docs.

@nmigen-issue-migration
Copy link
Author

@nmigen-issue-migration nmigen-issue-migration commented Feb 7, 2019

Comment by eilims
Thursday Feb 07, 2019 at 06:43 GMT


Howdy!
For the global section on the installation of nmigen it would be good to add that the pip install command (if used) requires pip3 to properly install using setup.py. The current pip command: pip install git+https://github.com/m-labs/nmigen.git references python2 (at least on my machine (Ubuntu)) and fails to install due to the Python 3.6+ requirement.

Following these commands will correctly install the package:

  1. sudo apt-get install python3-pip
    To get the python3 version of pip
  2. pip3 install git+https://github.com/m-labs/nmigen.git
    Install nmigen using python3 without the taint of python2

This ensures that all packages can be found by the installer as pulling the repository and attempting to run the setup.py locally using python3 will result in missing packages (notably Python.h)

I was installing nmigen today and may have pulled out a hair or two and just wanted to make everyone's life a little easier!

@nmigen-issue-migration
Copy link
Author

@nmigen-issue-migration nmigen-issue-migration commented Feb 7, 2019

Comment by sam-falvo
Thursday Feb 07, 2019 at 20:48 GMT


Thank you for the feedback. I'd missed this. I'll probably be able to commit your recommended changes in a day or two, unless by some miracle I find the free time later tonight. :)

@nmigen-issue-migration
Copy link
Author

@nmigen-issue-migration nmigen-issue-migration commented Feb 21, 2019

Comment by sam-falvo
Thursday Feb 21, 2019 at 16:21 GMT


I decided to make the change without mentioning Ubuntu or Debian specifically. Not all Linux distributions use a distinct pip3 executable (I use Void Linux, for instance, and at least within a virtualenv configured to use Python 3, pip and pip3 are identical executables). I'll be pushing the change shortly. Apologies for the long latency; the start-up that I work for has had me well and truly burnt out for a while.

@nmigen-issue-migration
Copy link
Author

@nmigen-issue-migration nmigen-issue-migration commented Feb 21, 2019

Comment by peteut
Thursday Feb 21, 2019 at 16:45 GMT


@sam-falvo It should not be possible to install nMigen on unsupported Python versions anyway, should be constrained using python_requires = ~=3.6,~=3.7.

[1] https://packaging.python.org/guides/distributing-packages-using-setuptools/#python-requires

@nmigen-issue-migration
Copy link
Author

@nmigen-issue-migration nmigen-issue-migration commented Feb 21, 2019

Comment by sam-falvo
Thursday Feb 21, 2019 at 17:06 GMT


Wouldn't >=3.6 be preferred?

On Thu, Feb 21, 2019 at 8:45 AM Alain Péteut notifications@github.com
wrote:

@sam-falvo https://github.com/sam-falvo It should not be possible to
install nMigen on unsupported Python versions anyway, should be
constrained using python_requires = =3.6,=3.7.

[1]
https://packaging.python.org/guides/distributing-packages-using-setuptools/#python-requires


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
m-labs/nmigen#22 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/ABbP-IMtIFFe6Ng_O8JssDnomiJPCihdks5vPs0igaJpZM4ZylP2
.

--
Samuel A. Falvo II

@nmigen-issue-migration
Copy link
Author

@nmigen-issue-migration nmigen-issue-migration commented Feb 21, 2019

Comment by peteut
Thursday Feb 21, 2019 at 17:22 GMT


Wouldn't >=3.6 be preferred?

On Thu, Feb 21, 2019 at 8:45 AM Alain Péteut @.***> wrote: @sam-falvo https://github.com/sam-falvo It should not be possible to install nMigen on unsupported Python versions anyway, should be constrained using python_requires = =3.6,=3.7. [1] https://packaging.python.org/guides/distributing-packages-using-setuptools/#python-requires — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#22 (comment)>, or mute the thread https://github.com/notifications/unsubscribe-auth/ABbP-IMtIFFe6Ng_O8JssDnomiJPCihdks5vPs0igaJpZM4ZylP2 .
-- Samuel A. Falvo II

Indeed, >=3.6. Sorry for that.

@whitequark
Copy link
Member

@whitequark whitequark commented Jul 2, 2020

@sam-falvo Thank you for your effort. Ultimately I decided to completely rewrite the manual for nMigen, with both different style and end goals. The current (quite unfinished but already useful) manual is available at https://nmigen.info/nmigen/.

@eilims You might find the new installation page useful: https://nmigen.info/nmigen/latest/install.html

@whitequark whitequark closed this Jul 2, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
2 participants