Added typestring parser checker #1402

Nikhil0504 · 2021-04-24T13:38:28Z

Changelog / Overview

Added typestring-parser

Motivation

Now the contributors can check if their typestrings are correct more easily.

Explanation for Changes

This could lessen the reviews and can PR's can be done faster. For first-timers, this would save them a lot of time as they could check here directly without going through many websites on whether they are correct or not.

Testing Status

Further Comments

Closes #1394

Checklist

I have read the Contributing Guidelines
I have written a descriptive PR title (see top of PR template for examples)
I have written a changelog entry for the PR or deem it unnecessary
My new functions/classes either have a docstring or are private
My new functions/classes have tests added and (optional) examples in the docs
My new documentation builds, looks correctly formatted, and adds no additional build warnings

Reviewer Checklist

The PR title is descriptive enough
The PR is labeled correctly
The changelog entry is completed if necessary
Newly added functions/classes either have a docstring or are private
Newly added functions/classes have tests added and (optional) examples in the docs
Newly added documentation builds, looks correctly formatted, and adds no additional build warnings

Nikhil0504 · 2021-04-24T13:50:47Z

@MrMallIronmaker can you check what the error is?

markromanmiller · 2021-04-24T14:43:16Z

Nikhil - Thanks for the work you've been doing with the repo the past couple of days. I wanted to mention a couple things about communication, though.

First, I don't think I needed a ping on this issue. While I did reply to the original Discord comment, I don't have any special knowledge about doctypes or the module. Any member of the dev team could be equally suited for this question.

Second, I expected a more informative question than "can you check what the error was." Depending on how much experience you have with Github, CI, RTD, etc, I think good questions would be "How can I see what the problem was?" or "Why is RTD trying to run the code in my docs?" or "It works on my machine, why is it different here?" All of these questions are much more specific, so they show that you've put in the legwork in trying to teach yourself, they inform me a lot better about your experience so far, and they help me compose a better answer to your question.

I do appreciate your enthusiasm, and this message is one of those social norms to learn as you're getting started with FOSS and community software development, and in particular development with Manim.

docs/source/contributing/documentation.rst

Nikhil0504 · 2021-04-25T06:46:02Z

I am actually getting an error because typestring-parser isn't being recognized in the docs as it is not being downloaded via pip.
Maybe someone can tell where should I put this package to be installed as it is not being installed from requirements.txt

Logs:

File "contributing/documentation.rst", line ?, in default
Failed example:
    from typestring_parser import parse
Exception raised:
    Traceback (most recent call last):
      File "/opt/hostedtoolcache/Python/3.7.10/x64/lib/python3.7/doctest.py", line 1337, in __run
        compileflags, 1), test.globs)
      File "<doctest default[0]>", line 1, in <module>
        from typestring_parser import parse
    ModuleNotFoundError: No module named 'typestring_parser

markromanmiller · 2021-04-25T14:37:50Z

Oh sorry, I realized I misspoke in my message earlier. I still think we should not add a dependency for a single section of the docs. The solution would not be live code to be run by the docstring engine but rather a handful of examples in the text itself.

For future reference, the right place to add it to the list is the pyproject.toml file.

markromanmiller

This addition looks good, let me know when you address the smaller changes.

markromanmiller · 2021-04-26T21:47:59Z

docs/requirements.txt

@@ -2,4 +2,4 @@ Sphinx==3.1.2
 furo
 recommonmark>=0.5.0
 sphinx-copybutton
-sphinxext-opengraph


It looks like this line didn't actually change; can you checkout the old requirements.txt so that this file doesn't have a change here?

docs/source/contributing/documentation.rst

markromanmiller · 2021-04-27T20:00:33Z

It looks like "testsetup" gets rendered into the document. Is this intentional?

RickyC0626 · 2021-04-27T20:07:25Z

It looks like "testsetup" gets rendered into the document. Is this intentional?

I don't think so, maybe removing Example will help.

docs/source/contributing/documentation.rst

markromanmiller

lgtm, i can squash and merge this in a bit

naveen521kk · 2021-04-28T14:59:16Z

docs/source/contributing/documentation.rst

+    >>> parse('int')
+    <class 'int'>
+    >>> parse('int or str')
+    typing.Union[int, str]
+    >>> parse('list of str or str')
+    typing.Union[typing.List[str], str]
+    >>> parse('list of (int, str)')
+    typing.List[typing.Tuple[int, str]]


this doesn't render properly with syntax highlighting

No let's not have it as it just makes the CI fail.

I would think there's some way to have a code block highlighted in the documentation, but not run - right? @naveen521kk

If not, this is definitely not a breaking change.

@Nikhil0504 I know how to do this without breaking things, but I want you to search or rather explore to find things out.
You can easily add syntax highlighting, browse through though how it is done in other parts of manim itself or look at the reStructuredText Primer from sphinx or search the internet.

Hey, I tried fixing the issue and couldn't find a solution for it. Is it possible for you to fix it?

Take a look at how https://docs.manim.community/en/latest/tutorials/quickstart.html does it

markromanmiller

requesting changes to enable syntax highlighting

Nikhil0504 · 2021-05-01T14:25:20Z

Fixed the code block issue at last.

https://manimce--1402.org.readthedocs.build/en/1402/contributing/documentation.html

naveen521kk · 2021-05-01T14:31:00Z

docs/source/contributing/documentation.rst

+   be accessed by first downloading it via ``pip`` - ``pip install typestring-parser`` and
+   then using ``from typestring_parser import parse``.
+
+.. code-block:: bash


how is the below code bash? It's python.

We use it under bash or cmd so it's not python right?

Again, see how this is done in other places. It's obviously not bash.

I made another change and checked locally which passed but over here the pre commit is failing

Nikhil0504 · 2021-05-02T10:33:15Z

All checks have passed and I removed the bash Code which was changed to a doctest block

docs/source/contributing/documentation.rst

naveen521kk · 2021-05-02T14:44:29Z

docs/source/contributing/documentation.rst

@@ -318,6 +318,24 @@ elements that are either a ``str`` or ``None``;
 ``(str, int)``; ``Tuple[:class:`int`, ...]`` for a tuple of variable
 length with only integers.

+.. note::
+   As a helper for tool for typesets, you can use ``typestring-parser``: https://github.com/Dominik1123/typestring-parser 


rather than pasting the link near typestring-parser make that clickable.

markromanmiller

docs are looking good to me.

naveen521kk

sgtm

markromanmiller · 2021-05-07T04:53:14Z

@Nikhil0504 we can't merge until it's up-to-date, and we can't update the branch. Did you disable edits from maintainers on this branch?

Nikhil0504 · 2021-05-07T05:12:20Z

I gave the access and made the branch up to date.

Nikhil0504 added 3 commits April 24, 2021 19:02

Added typestring parser checker

796dfa7

Updated some errors

7b641c0

Update

a960bac

kolibril13 reviewed Apr 25, 2021

View reviewed changes

docs/source/contributing/documentation.rst Outdated Show resolved Hide resolved

Nikhil0504 added 2 commits April 25, 2021 12:27

Checking if installation wokrs

91724ea

Trying again

de6c15a

Nikhil0504 added 5 commits April 25, 2021 20:55

Updates

5af2882

Another Update

0054cf1

Update pyproject.toml

7d29612

Update documentation.rst

9d9b2b1

Merge branch 'master' into typestring_parser

da941c2

markromanmiller requested changes Apr 26, 2021

View reviewed changes

Update requirements.txt

b3301f6

kolibril13 added the documentation Improvements or additions to documentation label Apr 27, 2021

behackl reviewed Apr 27, 2021

View reviewed changes

docs/source/contributing/documentation.rst Outdated Show resolved Hide resolved

Update

279e718

markromanmiller approved these changes Apr 28, 2021

View reviewed changes

naveen521kk reviewed Apr 28, 2021

View reviewed changes

Nikhil0504 added 3 commits April 28, 2021 20:40

syntax highlighting should work?

c1f868c

Update

0d194c8

Reverted back

611cfcd

markromanmiller requested changes Apr 28, 2021

View reviewed changes

Nikhil0504 added 2 commits April 29, 2021 17:53

Will this work?

9074967

Update

d83e9fc

Nikhil0504 added 6 commits April 29, 2021 18:31

Still trying

5c79f5e

Update

71f6de8

Update again

23de171

Fixed the indentation issues

9b33fd9

Update

9a8dc83

updated code block

f6707ff

naveen521kk reviewed May 1, 2021

View reviewed changes

Nikhil0504 added 4 commits May 1, 2021 20:16

Update again

f092a6d

Update

d6036a0

Update again with blackendocs

45bd43d

Update documentation.rst

2d03500

naveen521kk reviewed May 2, 2021

View reviewed changes

docs/source/contributing/documentation.rst Outdated Show resolved Hide resolved

Added skip option to doctests

443318c

naveen521kk reviewed May 2, 2021

View reviewed changes

docs/source/contributing/documentation.rst Outdated Show resolved Hide resolved

naveen521kk reviewed May 2, 2021

View reviewed changes

docs/source/contributing/documentation.rst Outdated Show resolved Hide resolved

Added the link

09f0157

naveen521kk requested changes May 2, 2021

View reviewed changes

Update link

c40b742

markromanmiller approved these changes May 3, 2021

View reviewed changes

naveen521kk approved these changes May 3, 2021

View reviewed changes

Merge branch 'master' into typestring_parser

a8cd95b

markromanmiller merged commit 1746073 into ManimCommunity:master May 7, 2021

Nikhil0504 deleted the typestring_parser branch May 7, 2021 11:40

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Added typestring parser checker #1402

Added typestring parser checker #1402

Nikhil0504 commented Apr 24, 2021 •

edited by friedkeenan

Nikhil0504 commented Apr 24, 2021

markromanmiller commented Apr 24, 2021

Nikhil0504 commented Apr 25, 2021 •

edited

markromanmiller commented Apr 25, 2021

markromanmiller left a comment

markromanmiller Apr 26, 2021

markromanmiller commented Apr 27, 2021

RickyC0626 commented Apr 27, 2021

markromanmiller left a comment

naveen521kk Apr 28, 2021

Nikhil0504 Apr 28, 2021

markromanmiller Apr 28, 2021

naveen521kk Apr 28, 2021

Nikhil0504 Apr 30, 2021

Darylgolden Apr 30, 2021

markromanmiller left a comment

Nikhil0504 commented May 1, 2021 •

edited

naveen521kk May 1, 2021

Nikhil0504 May 1, 2021

naveen521kk May 1, 2021

Nikhil0504 May 1, 2021

Nikhil0504 commented May 2, 2021

naveen521kk May 2, 2021

markromanmiller left a comment

naveen521kk left a comment

markromanmiller commented May 7, 2021

Nikhil0504 commented May 7, 2021

Added typestring parser checker #1402

Added typestring parser checker #1402

Conversation

Nikhil0504 commented Apr 24, 2021 • edited by friedkeenan

Changelog / Overview

Motivation

Explanation for Changes

Testing Status

Further Comments

Checklist

Reviewer Checklist

Nikhil0504 commented Apr 24, 2021

markromanmiller commented Apr 24, 2021

Nikhil0504 commented Apr 25, 2021 • edited

markromanmiller commented Apr 25, 2021

markromanmiller left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

markromanmiller commented Apr 27, 2021

RickyC0626 commented Apr 27, 2021

markromanmiller 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

markromanmiller left a comment

Choose a reason for hiding this comment

Nikhil0504 commented May 1, 2021 • edited

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

Nikhil0504 commented May 2, 2021

Choose a reason for hiding this comment

markromanmiller left a comment

Choose a reason for hiding this comment

naveen521kk left a comment

Choose a reason for hiding this comment

markromanmiller commented May 7, 2021

Nikhil0504 commented May 7, 2021

Nikhil0504 commented Apr 24, 2021 •

edited by friedkeenan

Nikhil0504 commented Apr 25, 2021 •

edited

Nikhil0504 commented May 1, 2021 •

edited