KenKundert / inform

Library of print utilities used when outputting messages for the user from command-line programs.
https://inform.readthedocs.io
MIT License
7 stars 1 forks source link

Add `parse_range()` and `format_range()` functions. #3

Closed kalekundert closed 4 years ago

kalekundert commented 4 years ago

These functions are for converting between with strings like "1-3,5" and the sets they represent, e.g. {1,2,3,5}. This is useful because it's easy for humans to read and write these strings.

KenKundert commented 4 years ago

The tests are failing with the message:

    ValueError: line 855 of the docstring for inform.py has inconsistent leading whitespace: '    """'

I am not sure what is going on as line 855 does not appear to be a docstring.

Could you make the following enhancements:

  1. Provide description of the functions in the user manual section of the documentation.
  2. Provide an example of how the cast and range arguments would be used
  3. Add tests that include the cast and range arguments.

Also, in the description of the cast argument you say it is a function that casts to a 'sensible type'. Could you give more detail. In this context I don't know what 'sensible' means.

coveralls commented 4 years ago

Coverage Status

Coverage increased (+0.2%) to 94.83% when pulling 425876ced278e52a97d143549606d2614b092568 on kalekundert:master into ec42df76e86db1b37ca02db9a0744e4b49b9ab77 on KenKundert:master.

kalekundert commented 4 years ago

Hopefully this commit addresses your comments. I'm not sure what you meant about testing the cast and range arguments, though; there are already lots of tests for both arguments.

I'm not sure why the build isn't passing for python 3.5, but I don't think it's anything related to my code. It seems like some sort of version issue with pytest.

For what it's worth, I don't think the current distinction between the API docs and the "User's Guide" makes sense. The two sections are very redundant. I think it be better to reorganize a bit:

I think that would make the user's guide a more useful entry point for new users, while making the docs easier to write and maintain. That's just my opinion, though, don't feel obligated to do anything.

Also, if/when you merge this PR, can you make a new release?