Neaten up README.md a little #235

kaapstorm · 2024-02-14T00:19:36Z

Addresses a few nits:

Uses consistent spelling for "Python" and "CommCare HQ"
Drops a broken link
Fixes JSON and Python errors (except for one undefined variable that I couldn't figure out at a glance)
Simplifies virtualenv instructions a little
Applies recommended table formatting
Updates GitHub workflow to run tests against current Python releases

This is a documentation only change.

mkangia

Looks great! Thank for you this.

Left just minor queries.

mkangia · 2024-02-14T06:02:24Z

.github/workflows/test.yml

@@ -31,7 +31,7 @@ jobs:
          - 5432:5432
    strategy:
      matrix:
-        python-version: [3.7, 3.8, 3.9, '3.10', '3.11']
+        python-version: [3.8, 3.9, '3.10', 3.11, 3.12]


nit: '3.10' instead of 3.10?

The quotes are required for 3.10 because without them YAML evaluates it as a float with the value 3.1. The others are fine.

oh wow, can we keep all as strings then?

mkangia · 2024-02-14T06:03:40Z

README.md


 ```shell
 $ python --version
 $ python3 --version
 ```
-If python is installed, all of its available versions would be listed.
+If Python is installed, its version will be listed.


nit:
version will be ~~listed~~ shown

mkangia · 2024-02-14T06:04:26Z

README.md


-If python isn't installed, Install Python 3.8 from [this link](https://www.python.org/downloads/). 
+If Python isn't installed, [download and install](https://www.python.org/downloads/) the latest release. 


should we say install any version from 3.8 to 3.12?
If the latest version is above 3.12, It could lead to unexpected errors?

mkangia · 2024-02-14T06:06:27Z

README.md


 More about virtualenvs on https://docs.python.org/3/tutorial/venv.html

 Setup a virtual environment using:

 ```shell
-$ python3.8 -m venv .commcare-export-venv # update python version for the one installed
+$ python3 -m venv .venv


Any reason to change that back to venv?
I had made that change earlier to make the name more specific and avoid conflicts with any other setup since that is the name suggested by virtualenv documentation by default. We use the same name for CommCareHQ, venv.

Personally, I like "venv". (Some people seem to like to hide the directory.) I'll change it.

I am happy with venv if you wish to keep that.

mkangia · 2024-02-14T06:08:27Z

README.md

-export HQ_API_KEY=<apikey>
+```shell
+$ export HQ_USERNAME=<username>
+$ export HQ_API_KEY=<apikey>


nit:
Should not we generally avoid "$"?
Whenever I need to simply copy the command and run it, I need to remove that $ sign each time after pasting that into terminal.

I added them in here to be consistent with the rest of the document (although I've just noticed I missed two others).

Generally, I have found it useful to indicate the prompt to make it clear what kind of prompt it is. e.g.

$ psql -U commcarehq =# \l List of databases ... =# \q $ python3 >>> print('Hello world.') Hello world.

I have found that often writers will leave out the prompt so consensus is not universal, but that more often than not, they include it. Certainly across Dimagi docs, we seem to include it most of the time.

mkangia · 2024-02-14T06:09:23Z

README.md


 results = query.eval(BuiltInEnv() | CommCareHqEnv(api_client) | JsonPathEnv())

 if len(list(env.emitted_tables())) > 0:
-    # with writers.Excel2007TableWriter("excel-output.xlsx") as writer:
-    with writers.StreamingMarkdownTableWriter(sys.stdout) as writer:
+    # with Excel2007TableWriter("excel-output.xlsx") as writer:


nit: could also remove that comment I believe.

mkangia · 2024-02-14T06:10:28Z

README.md

-| `+, -, *, //, /, >, <, >=, <=` | Standard Math                                                                  |                                  |
+| Function                       | Description                                                                  | Example Usage                    |
+|--------------------------------|------------------------------------------------------------------------------|----------------------------------|
+| `+, -, *, //, /, >, <, >=, <=` | Standard Math                                                                |                                  |


nit: Looks like this needs some more alignment correction.

.github/workflows/test.yml

mkangia · 2024-02-14T06:11:59Z

README.md

@@ -71,6 +71,7 @@ $ venv
 Install CommCare Export via `pip`

 ```shell
+$ pip install setuptools


nit: Should we make a comment that this is for version 3.12?

I just tested this in Python 3.12. Turns out we don't need setuptools if we install using pip install commcare-export. I'll drop this line.

mkangia · 2024-02-14T18:36:59Z

README.md

@@ -42,23 +43,23 @@ More about virtualenvs on https://docs.python.org/3/tutorial/venv.html
 Setup a virtual environment using:

 ```shell
-$ python3 -m venv .venv
+$ python3 -m venv venv


oh, we are not keeping the folder hidden anymore.

Non blocking but for someone who does not understand these things well, they would get really confused of two "venv"s, about which one is command and which one is a folder.

Oh! I completely misunderstood your comment,

Any reason to change that back to venv?
I had made that change earlier to make the name more specific and avoid conflicts with any other setup since that is the name suggested by virtualenv documentation by default. We use the same name for CommCareHQ, venv.

I thought you were asking me to change .venv to venv in order to be consistent with its name in CommCare HQ. Sorry for my confusion.

avoid conflicts

virtualenvwrapper puts all your virtual environments in one place, so it's important to give them unique names. But the Standard Library's venv is simpler, and doesn't have a special place for all your virtual environments. In my experience, most people just use a subdirectory in the repository's root named "venv", so there is no risk of conflicts. This convention is used in GitHub's template .gitignore file for Python projects. I've just applied the same convention here.

kaapstorm added 6 commits February 13, 2024 17:03

Test on current Python releases

b0f1e81

Neaten up README.md a little

0b134d1

python might not work but python3 always will

4ff0e40

Fix nearly all Python and JSON errors

ec54ac3

Fix table formatting

b105c0f

"CommCare HQ"

e337ade

kaapstorm requested review from mkangia and SmittieC February 14, 2024 00:19

kaapstorm added 4 commits February 14, 2024 00:30

Fix raw string

18fec75

Install setuptools for Python 3.12

c4b59bc

Update README.md for Python 3.12

9fdfbaa

Update versions in setup.py

4f4bb30

mkangia reviewed Feb 14, 2024

View reviewed changes

kaapstorm added 6 commits February 14, 2024 12:37

Use "venv" directory for virtualenv

84fe33b

setuptools not required

3974ae6

Fix column width

e78dddb

Specify versions that are tested

dff0651

Quote version numbers

f7bc3f5

Drop comment

a2f0bf6

mkangia reviewed Feb 14, 2024

View reviewed changes

mkangia approved these changes Feb 14, 2024

View reviewed changes

kaapstorm merged commit 6f7a0c8 into master Feb 15, 2024
5 checks passed

kaapstorm deleted the nh/readme branch February 15, 2024 00:59

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Neaten up README.md a little #235

Neaten up README.md a little #235

kaapstorm commented Feb 14, 2024

mkangia left a comment

mkangia Feb 14, 2024

kaapstorm Feb 14, 2024

mkangia Feb 14, 2024

kaapstorm Feb 14, 2024

mkangia Feb 14, 2024

kaapstorm Feb 14, 2024

mkangia Feb 14, 2024

kaapstorm Feb 14, 2024

mkangia Feb 14, 2024

kaapstorm Feb 14, 2024

mkangia Feb 14, 2024

kaapstorm Feb 14, 2024

mkangia Feb 14, 2024

kaapstorm Feb 14, 2024

mkangia Feb 14, 2024

kaapstorm Feb 14, 2024

mkangia Feb 14, 2024

kaapstorm Feb 14, 2024

mkangia Feb 14, 2024

kaapstorm Feb 14, 2024

kaapstorm Feb 14, 2024

mkangia Feb 14, 2024

kaapstorm Feb 15, 2024

mkangia Feb 15, 2024


		If python isn't installed, Install Python 3.8 from [this link](https://www.python.org/downloads/).
		If Python isn't installed, [download and install](https://www.python.org/downloads/) the latest release.

Neaten up README.md a little #235

Neaten up README.md a little #235

Conversation

kaapstorm commented Feb 14, 2024

mkangia left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment