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

Start adding docs to readme #33

Merged
merged 23 commits into from
Nov 12, 2018
Merged

Start adding docs to readme #33

merged 23 commits into from
Nov 12, 2018

Conversation

saulshanabrook
Copy link
Contributor

@saulshanabrook saulshanabrook commented Nov 4, 2018

Thanks to a helpful discussion with @dcharbon on Friday, I have started outlining how this system works.

  • Finish writing
  • Think about if we should change names to make them more math (category theory)-ey.
  • Add replace_scan
  • Test readme code
  • Make notebook instead

Fixes #25

@saulshanabrook
Copy link
Contributor Author

saulshanabrook commented Nov 8, 2018

In this process, I realize I can add typing and register an operation very cleanly as a function!

Going from this:

class GetItem(matchpy.Operation):
    name = "GetItem"
    arity = matchpy.Arity(1, True)

# then in .pyi file
def GetItem(seq: CArray) -> CGetitem:
    ...

To this:

@operation
def GetItem(length: CContent, getitem: CGetitem) -> CArray:
    ...

And `operation is pretty simple as well:

T = typing.TypeVar("T", bound=typing.Callable)


def operation(fn: T) -> T:
    """
    Register a matchpy operation for a function. 

    The function should have a fixed number of args and one return.

    This can also be done manually, by typing the result of `matchpy.Operation.new`
    as a callable, but this is more fluent.
    """
    sig = inspect.signature(fn)
    return matchpy.Operation.new(fn.__name__, matchpy.Arity(len(sig.parameters), True))

@saulshanabrook
Copy link
Contributor Author

Trick is understanding that all this is just for compile time checking, not runtime.

@saulshanabrook saulshanabrook changed the title WIP: Start adding docs to readme Start adding docs to readme Nov 11, 2018
@saulshanabrook
Copy link
Contributor Author

OK this should be ready to merge. I have added some tests, but no CI yet. The tests just run some notebooks, run mypy and verify the readme works.

@saulshanabrook
Copy link
Contributor Author

Closes #25

@hameerabbasi
Copy link
Collaborator

Magic comments only work in the PR description, not in another comment. Might be wise to add it to the PR description.

@saulshanabrook
Copy link
Contributor Author

CI is working now on this and it is ready to merge.

Copy link
Collaborator

@hameerabbasi hameerabbasi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was wondering about the naming convention for MyPy types... Would it be better to have a typing module like Python?

uarray/uarray/core.py Outdated Show resolved Hide resolved
@saulshanabrook
Copy link
Contributor Author

@hameerabbasi good idea, I moved the types into separate files. How is that?

@hameerabbasi
Copy link
Collaborator

It's better, I was also wondering about the naming convention, what does the C signify? Is it necessary to have it? Python just has PascalCase convention, usually. Are you following the C++ way of prefacing type names with C?

It'd be nice to have all of them in a simple typing module, and we can make the rest of them private. Another way is to have them close to where they should be.

Examples of both: typing.List and collections.Iterable.

@saulshanabrook
Copy link
Contributor Author

C stands for Category. All the names will need to changed, i have gotten feedback from a couple people that the resemblance to Pythons naming (GetItem, Sequence) is confusing.

At least at the moment within this PR it is internally consistent that all compile time types are prefixed with C.

I expect the final look of how we write these to change.

I'll add a few lines to the readme to explain how they are currently.

@hameerabbasi
Copy link
Collaborator

Perhaps have a Category base class? It could come in useful either way.

@saulshanabrook
Copy link
Contributor Author

Sounds good. Added that.

@saulshanabrook
Copy link
Contributor Author

I am gonna merge this in so that I can share the current state of the repo with others for feedback more easily.

@saulshanabrook saulshanabrook merged commit 53c6910 into master Nov 12, 2018
@hameerabbasi hameerabbasi deleted the add-docs branch March 12, 2019 17:28
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.

2 participants