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

Fix single line docstring parsing error #68

Merged
merged 5 commits into from Nov 27, 2020
Merged

Fix single line docstring parsing error #68

merged 5 commits into from Nov 27, 2020

Conversation

michaellisitsa
Copy link
Contributor

What it fixes
Related to Issue #66 this allows a single line doc-string to be used when using the python decorator.
I read online that Regex needs to be compiled first, which can make it slower than the python string functions I adopted.

Limitations
I decided to keep it simple for my first pull request, in case the maintainer wants to style the code differently.

Testing conducted

  • created a new environment in conda and pip install streamlit
  • Forked and cloned the handcalcs repo
  • python setup.py install --user inside the cloned handcalcs directory. see bottom as the installation didn't behave quite right
  • Run the following script including writing 2 identical functions except with a single vs. double line doc-string
import streamlit as st
import handcalcs

@handcalcs.handcalc(override="long")
def addition1(val1,val2):
    """doc string doc string 2"""
    c = val1/val2
    return c

@handcalcs.handcalc(override="long")
def addition2(val1,val2):
    """doc string
    doc string 2"""
    c = val1/val2
    return c

latex1, vals1 = addition1(1,2)
st.write("Single Line docstring")
st.latex(latex1)

latex2, vals2 = addition2(a,b)
st.write("Multi Line docstring")
st.latex(latex2)

Output
image

Issues installing using python setup.py install
When running the above script additionX(val1,val2) was generating the following console outputs (which I usually don't get when I pip install handcalcs). The latex appears to still work though.

No match:  val1
div:  /
No match:  val2
Return:  [True, False, True, False]
op:  =
No match:  val1
div:  /
No match:  val2
Return:  [True, False, True, False]

@michaellisitsa
Copy link
Contributor Author

michaellisitsa commented Nov 24, 2020

Thankfully your testing is all automatic, and 1 test failed where """ was on a separate line.

Additional changes made
To ensure a string that equals """ on the first line is correctly categorised:

  • before running endswith() first clean:
    • tabs or whitespaces to left; and
    • whitespaces to right.
  • then let endswith only read from the 4th character. This either ignores the first 3 alphanumeric characters (harmless), or when string == """, it ignores these 3 characters and correctly determines there are no triple quotes at the end.

Additional code tested

@handcalcs.handcalc(override="long")
def addition3(val1,val2):
    """
    My doc string
    Line 2 of doc string
    """
    c = val1/val2
    return c

latex3, vals3 = addition3(1,2)
st.write("3 dashes on line by themselves")
st.latex(latex3)

Ouput
image

@ptmcg
Copy link

ptmcg commented Nov 24, 2020

textwrap.dedent is also good for stripping leading whitespace in a nice way vs. using lstrip:

    """
    This docstring has
        some indented text.
    """

using lstrip() loses the indentation - textwrap.dedent will preserve it.

@michaellisitsa
Copy link
Contributor Author

I don't think keeping the indentation on the doc-string is necessary, because handcalcs is just testing for its existence, then throwing it away.

I'll wait to see if Connor wants to integrate ''' and ' and ", though probably worth then pulling the logic out. A different function could determine the bool doc_string based on arguments line, doc_string.

…urce_to_cell to remove_imports_defs_and_globals
@michaellisitsa
Copy link
Contributor Author

michaellisitsa commented Nov 25, 2020

I added testing for the new _func_source_to_cell. I deduced how your testing works, but feel free to amend if incorrect:

  • Copied across the updated _func_source_to_cell function to test_handcalcs_file.py except I kept the differences you had in there already because they appear to be necessary for the testing to run:
    • check for import and global keywords which are unique to the cellX.py test files, so needed to make them work
    • different function name is used to above remove_imports_defs_and_globals
    • There was another difference on line 102 I couldn't figure out. It seemed extraneous because the subsequent if statement checks for not doc_string. Therefore I removed it:
elif doc_string:
            continue
  • Did not add any new cellX.py files, as this PR is specific to the decorator which is covered by func_1 func_2 etc.
  • Added func_3 function which is a duplicate of func_2 except that the doc-string is on a single line. This is the main test for the PR.

@connorferster
Copy link
Owner

Not too worried about all string literals being a possible doc string right now. I don't think I have ever actually seen anyone do that (but I am sure they are out there).

@connorferster
Copy link
Owner

Thank you very much, @michaellisitsa!

@connorferster connorferster merged commit f2a85b9 into connorferster:master Nov 27, 2020
connorferster added a commit that referenced this pull request Dec 13, 2020
Fix single line docstring parsing error
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.

None yet

3 participants