Readme tools samples by jtmoon79 · Pull Request #9 · NeilGirdhar/ipromise

jtmoon79 · 2022-08-21T06:22:54Z

revise README, add samples

Revise README.rst to use syntax sections recommended in
https://www.sphinx-doc.org/en/master/usage/restructuredtext/basics.html#sections

Improve README opening to better explain ipromise.

Add table of contents to README.

Revise README.rst line widths.

Add easier-to-read sample code, both in README and in python
files under samples/.

jtmoon79 · 2022-08-24T17:37:12Z

@NeilGirdhar any opinion on this PR?

README.rst

NeilGirdhar · 2022-08-24T17:40:26Z

README.rst


-This repository provides a Python base class, and various decorators for specifying promises relating to inheritance.
-It provides three inheritance patterns:
+This repository, ``ipromise``, provides a Python base class and


Either say "this repository" or "I Promise". Just choose one.

NeilGirdhar · 2022-08-24T17:41:42Z

README.rst

-This repository provides a Python base class, and various decorators for specifying promises relating to inheritance.
-It provides three inheritance patterns:
+This repository, ``ipromise``, provides a Python base class and
+decorators to guarantee inheritance "promises".


Okay, this is good, but no need for the quotations around "promises".

NeilGirdhar · 2022-08-24T17:42:41Z

README.rst

+
+* implementing
+* overriding
+* augmenting


Keep the commas and the "and". This is still a sentence.

NeilGirdhar · 2022-08-24T17:43:36Z

README.rst

+* overriding
+* augmenting
+
+Using the ``ipromise`` inheritance patterns can ensure an inheritance hierarchy


I think this para is redundant. It doesn't say anything the previous paragraph doesn't. "Build time" doesn't mean anything in Python either.

I really preferred this as it explains the mechanism by which ipromise works. Not all Python users may intuit how ipromise functions.

I can remove it if you'd still prefer it removed.

Fixed "build time".

NeilGirdhar · 2022-08-24T17:55:20Z

tools/flake8.sh

+#
+# run flake8 with project settings
+
+set -eu


What does this file add? I just do this:

poetry install poetry shell pflake8 .

I don't think we need complicated scripts for people who don't know how to use poetry.

I adjusted the install script help instructions to the general poetry steps.

The wrapper scripts handle the details for developers:

running from the correct path

explicitly specifying a config file

passing expected arguments

sanity check the program exists and can run --version

IME wrapper scripts like this greatly reduce confusion around running project tools.

Also, having these tool wrapper scripts is like a big hint "make sure to run these tools with these settings!".

Also, in my case, some poetry commands failed with what appears to be a poetry bug. I had to come up with some workarounds. For example, sometimes I would get

$ poetry install Updating dependencies Resolving dependencies... (4.1s) ValueError invalid literal for int() with base 10: b'' at .venv-3.9.13_ubuntu-22.04/lib/python3.9/site-packages/cachy/stores/file_store.py:65 in _get_payload 61│ 62│ with open(path, 'rb') as fh: 63│ contents = fh.read() 64│ → 65│ expire = int(contents[:10])

The slightly unusual command-lines in the "tools" scripts were what worked for me most often. My updates to this PR removed my uncommon approach.

Check out the updated "tools" scripts in my update push.
If you'd still like the "tools" scripts removed then I will remove them.

Removed scripts.

NeilGirdhar · 2022-08-24T17:55:45Z

tools/mypy.sh

+
+set -eu
+
+cd "$(dirname -- "${0}")/.."


Same, i don't think this file helps. I just do

mypy ipromise

Currently, I recommended two paths for mypy, ipromise and test. mypy passes.

Then just do mypy ipromise tests. You can add that to the readme?

NeilGirdhar · 2022-08-24T17:55:58Z

tools/pytest.sh

+set -eu
+
+cd "$(dirname -- "${0}")/.."
+


Same, no need for this.

NeilGirdhar · 2022-08-24T17:57:29Z

tools/rst-lint.sh

+set -eu
+
+cd "$(dirname -- "${0}")/.."
+


This is good information. Would you mind putting this line:

rst-lint --level info README.rst

into the readme itself? You could add information about runnning mypy. pflake8, etc. there too? Maybe create a new section?

There is currently section Development which refers to these scripts. Is that adequate?

Yes, sounds great. Would you mind adding this line to that section?

README.rst

jtmoon79 · 2022-08-25T04:24:09Z

I squashed the force-pushed the commits (which messes up the PR review history above 😕 but is a cleaner commit to master branch).

NeilGirdhar · 2022-08-25T14:47:03Z

@jtmoon79 Thanks, can you remove the scripts? I still see them.

jtmoon79 · 2022-08-25T19:31:46Z

@jtmoon79 Thanks, can you remove the scripts? I still see them.

Whoops, for some reason the git rm commit was dropped. Should be fixed now.

NeilGirdhar

Almost there

README.rst

NeilGirdhar · 2022-08-25T21:23:46Z

README.rst

+
+.. code-block:: shell
+
+    pip install restructuredtext_lint Pygments


This install should be in pyproject. If it isn't working for you, there's something wrong with your poetry setup.

NeilGirdhar · 2022-08-25T21:24:12Z

README.rst

+    python -m pip install ipromise
+
+***
+Use


Awesome, thanks.

NeilGirdhar · 2022-08-25T21:26:35Z

README.rst

+            print("MyClassAgumentedOnceAgain().f()")
            return super().f(**kwargs)
+
+    MyClassAgumentedOnce().f()


Okay that makes sense.

Revise README.rst to use syntax sections recommended in https://www.sphinx-doc.org/en/master/usage/restructuredtext/basics.html#sections Improve README opening to better explain `ipromise`. Add table of contents. Revise README.rst line widths. Add easier-to-read sample code, both in README and in python files under `samples/`.

jtmoon79 · 2022-08-26T20:19:19Z

@NeilGirdhar how about this round of updates?

NeilGirdhar

Thank you so much for this PR, and for your patience with the review!

NeilGirdhar reviewed Aug 24, 2022

View reviewed changes

NeilGirdhar reviewed Aug 25, 2022

View reviewed changes

NeilGirdhar approved these changes Aug 27, 2022

View reviewed changes

NeilGirdhar merged commit d05cbab into NeilGirdhar:master Aug 27, 2022


		.. code-block:: shell

		pip install restructuredtext_lint Pygments

Comments

Conversation

jtmoon79 commented Aug 21, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jtmoon79 commented Aug 24, 2022

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

NeilGirdhar Aug 24, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jtmoon79 Aug 24, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

jtmoon79 commented Aug 25, 2022

Uh oh!

NeilGirdhar commented Aug 25, 2022

Uh oh!

jtmoon79 commented Aug 25, 2022

Uh oh!

NeilGirdhar left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jtmoon79 commented Aug 26, 2022

Uh oh!

NeilGirdhar left a comment

Choose a reason for hiding this comment

Uh oh!

jtmoon79 commented Aug 21, 2022 •

edited

Loading

NeilGirdhar Aug 24, 2022 •

edited

Loading

jtmoon79 Aug 24, 2022 •

edited

Loading