Skip to content

ansys-templates edits to doc #39

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

Merged
merged 15 commits into from
Mar 28, 2022
Merged

ansys-templates edits to doc #39

merged 15 commits into from
Mar 28, 2022

Conversation

PipKat
Copy link
Member

@PipKat PipKat commented Mar 25, 2022

Kelly Fletcher tried to use this doc to set up a PyAnsys project for NSV doc and had problems because ansys-templates was noted instead of pyansys-templates. She also couldn't use pipx. I'll add a comment in the file to better explain.


.. code:: bash

python -m pip install --user pipx

Ensure that `pipx`_ is in your ``PATH`` by running:
Ensure that `pipx`_ is in your ``PATH`` with:

.. code:: bash

python -m pipx ensurepath
Copy link
Member Author

Choose a reason for hiding this comment

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

Kelly had trouble here. She could run python -m pip ensurepath but not use pipx. It could be she missed something though as she is new to process.

Copy link
Member

Choose a reason for hiding this comment

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

python -m pip ensurepath

Just to make sure, this should be python -m pipx ensurepath (not pip).

Copy link
Member Author

@PipKat PipKat Mar 25, 2022

Choose a reason for hiding this comment

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

That is what it says--but it didn't work for her. Kelly said she did install pipx as instructed but it didn't work for installing pyansys-templates. She received this error:
Error: File "C:\Users\kfletche\AppData\Roaming\Python\Python37\site-packages\packaging\utils.py", line 34, in canonicalize_name
value = _canonicalize_regex.sub("-", name).lower()
TypeError: expected string or bytes-like object

Copy link
Member

Choose a reason for hiding this comment

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

This error does not provide enough information... I recommend Kelly to open an issue on this repo pasting the full set of commands and the output she gets. If she is not familiar with this workflow, there is no problem on having a quick virtual meeting to investigating this @PipKat.

You can easily add new templates to ``ansys-templates``. Before adding a new
template, it is important that you read the CONTRIBUTING section.
You can easily add new PyAnsys templates. However, before doing so, it is important that
you read the CONTRIBUTING section.
Copy link
Member Author

Choose a reason for hiding this comment

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

Is this section yet to be added or will it be to the overall Contributing section in the PyAnsys Developer's Guide. I assume that it will someday have a contributing.rst file of its own that references the Contributing topic in the PyAnsys Developer's Guide. At that point, we should have the link text here be Contributing--not CONTRIBUTING.

Copy link
Member

Choose a reason for hiding this comment

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

Good catch on this one, @PipKat! I was planning to link this to some contributing section when available in the dev-guide. I guess that for the moment we can link this to https://dev.docs.pyansys.com/guidelines/dev_practices.html#contributing-through-github

Copy link
Member

Choose a reason for hiding this comment

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

PipKat and others added 2 commits March 25, 2022 15:56
Use pipx with literal tagging in heading
Use pip in heading with literal tagging

Co-authored-by: Dominik Gresch <[email protected]>
Fix underline that was too short.
Fix another underline that was too short.
Fix bullet with unexpected indent and missing blank line ending list
Copy link
Member

@jorgepiloto jorgepiloto left a comment

Choose a reason for hiding this comment

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

Thanks for the deep review @PipKat! I left some comments. Most of those are related with the fact that the repo is named pyansys-templates but the tool provided is named ansys-templates.

@akaszynski
Copy link
Contributor

Most of those are related with the fact that the repo is named pyansys-templates but the tool provided is named ansys-templates.

Should we make it consistent? I can see how this might be confusing to users.

@jorgepiloto
Copy link
Member

Should we make it consistent? I can see how this might be confusing to users.

I thought about this too... From my point of view, calling it pyansys-templates is more intuitive for users but might restrict its usage to Python packages only. Anyway, I do not think we start implementing non-Python packages in the short term.

When naming this, I just imposed it to be ansys-templates to behave similar as the Python namespace we are following in the PyAnsys ecosystem: from ansys.templates import ...

@akaszynski
Copy link
Contributor

Should we make it consistent? I can see how this might be confusing to users.

I thought about this too... From my point of view, calling it pyansys-templates is more intuitive for users but might restrict its usage to Python packages only. Anyway, I do not think we start implementing non-Python packages in the short term.

When naming this, I just imposed it to be ansys-templates to behave similar as the Python namespace we are following in the PyAnsys ecosystem: from ansys.templates import ...

I agree with the reasoning regarding the python library name, but we probably should change this repo name to ansys-templates for consistency.

@jorgepiloto
Copy link
Member

I agree with the reasoning regarding the python library name, but we probably should change this repo name to ansys-templates for consistency.

I agree with this, let's rename it to ansys-templates.

@jorgepiloto
Copy link
Member

Done!

You can easily add new templates to ``ansys-templates``. Before adding a new
template, it is important that you read the CONTRIBUTING section.
You can easily add new PyAnsys templates. However, before doing so, it is important that
you read the CONTRIBUTING section.
Copy link
Member

Choose a reason for hiding this comment

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

@jorgepiloto jorgepiloto changed the title pyansys-templates edits to doc ansys-templates edits to doc Mar 28, 2022
Copy link
Member

@jorgepiloto jorgepiloto left a comment

Choose a reason for hiding this comment

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

Approving this @PipKat, thanks for the deep review.

We agreed on renaming the repository as ansys-templates so it matches the name of the CLI tool provided.

@akaszynski
Copy link
Contributor

This is good to merge @jorgepiloto.

@jorgepiloto jorgepiloto merged commit c1ac3b9 into main Mar 28, 2022
@jorgepiloto jorgepiloto deleted the doc/general_doc_edits branch March 28, 2022 09:25
@jorgepiloto
Copy link
Member

jorgepiloto commented Mar 28, 2022

Merging this! Thanks for deep review, @PipKat 🚀 We still need to investigate the installation issue Kelly reported.

@PipKat
Copy link
Member Author

PipKat commented Mar 28, 2022

@jorgepiloto Kelly did say that she was able to generate her first local doc build for her solution, but was still very much in the learning process. I don't think she's worried about this issue anymore, but I will tell her to reach out to you if she has any problems later.

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.

4 participants