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

Discuss duck typing vs. assertions vs. contracts #56

Closed
sdrave opened this issue Sep 27, 2013 · 9 comments
Closed

Discuss duck typing vs. assertions vs. contracts #56

sdrave opened this issue Sep 27, 2013 · 9 comments
Milestone

Comments

@sdrave
Copy link
Member

sdrave commented Sep 27, 2013

No description provided.

@sdrave sdrave modified the milestones: 0.3, 0.4 Sep 3, 2014
@sdrave
Copy link
Member Author

sdrave commented Jun 29, 2015

@renemilk, @ftalbrecht, with PEP484 accepted, Python now has an official syntax for static type annotations which is based on mypy.
The syntax of pycontracts is different (see here).

Since none of the two is currently used by pyMOR, I propose to use the PEP-484 syntax if we decide to have type annotations at all. The only reason to use pycontracts would be that pycontracts might offer an additional feature which we really want to have. However, since @renemilk suggests that our current assertions like op1.source == op2.range cannot be expressed with pycontracts, I do not think there is such a feature.

@sdrave
Copy link
Member Author

sdrave commented Jun 29, 2015

In general, I believe that, if at all, we should opt for optional static typing which only raises warnings, not errors.

@ftalbrecht
Copy link
Contributor

If I understand the pep correctly, it does not even aim at any runtime warning or error, merely at improved readability and offline linting (any warning would have to be provided by us, or does some package like mypy already do that?).

After a quick glance I find the syntax acceptable, if we were to adopt it, and would agree to drop pycontracts.

@sdrave
Copy link
Member Author

sdrave commented Jun 29, 2015

@ftalbrecht, you understand correctly that the PEP is intended for static analysis. mypy does this already, there is nothing that needs to be done by pyMOR. (Checking if the spaces fit would still have to be performed manually using assertions.)

@renefritze
Copy link
Member

I'm pro using only PEP-484. I'll remove pycontracts in a branch.

@sdrave
Copy link
Member Author

sdrave commented Jul 4, 2015

Ok, so we will add PEP484-style annotations in the future. Question remains, how many assertions we want to have in pyMOR. For instance, do we want to check in all apply methods that

assert U in self.source

On the plus side, we can easily check if a malformed vector is passed to the operator. On the other hand, this might get in your way: For instance, I have a DUNE code where the discretization object is part of the subtype as it holds the necessary function space object to create new vectors. Thus, such an assertion would forbid me to pass a vector of one discretization to an operator of another discretization, even if it makes perfect sense.

BTW, is there a way to turn assertions into warnings?

@ftalbrecht
Copy link
Contributor

I agree that the check might seem to get in your way in your example. On the other hand, "makes perfect sense" only goes so far. In practice, you will not be able to pass a vector from a discretization compiled into one shared object to another discretization compiled into another shared object (even if the original C++ types were identical). At least with pybindgen this is not possible and I doubt that boost-python will differ on that one. So the assertion is valid in that case, since your code would blow up in that case anyway a few lines later.

To gracefully handle the situation we would have to reinterpret the vector with respect to the other discretization, which will probably be realized by wrapping the underlying array into the new type:

if U not in self.source:
    # try to wrap U
    # ...
assert U in self.source

So all in all I think the above assertion is fine and should be there.

@sdrave
Copy link
Member Author

sdrave commented Jul 5, 2015

@ftalbrecht, I am doing this already. The VectorArrays have the same type, only the discretization instance they have as subtype differs. (And the discretizations are only needed to pull out the GridFunctionSpace which is needed to instantiate the dof vector - in my case, only extracting the dimension.)

As long as you are the implementor of apply, there is no problem here, anyway. Just leave out the assert statement. Things start getting complicated if you want to use generic constructions such as LincombOperator or Concatenation where apply is already implemented.

@sdrave
Copy link
Member Author

sdrave commented Sep 29, 2015

Ok, so it still remains to decided how many and which kind of assertions we want to have. I have created a new issue for this (#153), postponing the decision to 0.5.
Closing this issue.

@sdrave sdrave closed this as completed Sep 29, 2015
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

No branches or pull requests

3 participants