If class field not assigned to in Python constructor, should be nullable #317
Comments
JSAbrahams
added
the
bug: check
JSAbrahams
added a commit
that referenced
this issue
May 27, 2022
JSAbrahams
added a commit
that referenced
this issue
May 28, 2022
* Don't include Python init in GenericClassDef - Made note of #311 * Don't include Python init in GenericClassDef - Made note of #314 - Made note of #315 - Made note of #316 - Made note of #317 - Made note of #318 * Check that typed assign with expr does have type - Made note of #319 * Test functionality of class from AST - Type definition (and aliases) have self argument - Make self in class have class as type - Remove args from GenericParent, it doesn't do anything. - Function arguments from python source is mutable. The application logic in class from AST is currently very convoluted. Could use a rewrite. * Test that py and mamba class can have generics TrueName now takes generic arguments from type. Now context also stores class generics from Mamba classes. It may very well be that this explains some weird behaviour we had around generics (in that they didn't work in most cases). In a future release we should test this further since I'm sure there are some cases we've missed. - Made note of #320 * Add optional to Name in generate Always print out generics in alphabetical order. This ensures that the output is easy to predict, we are not dependent on the whims of the iterator over the set, possibly preventing flaky tests. Make annotating output the default in tests as well. As we start to implement new features and debug the checking stage we should then automatically see old tests failing as well. * Make sorting of generics case-insensitive Add new description to README * Verify current unknown generic parent behaviour Currently, no check is done that a parent generic exists until the class is actually used. Perhaps in future this behaviour should be changed, however.
JSAbrahams
added a commit
that referenced
this issue
May 28, 2022
JSAbrahams
added a commit
that referenced
this issue
Jun 7, 2022
* Don't include Python init in GenericClassDef - Made note of #311 * Don't include Python init in GenericClassDef - Made note of #314 - Made note of #315 - Made note of #316 - Made note of #317 - Made note of #318 * Check that typed assign with expr does have type - Made note of #319 * Test functionality of class from AST - Type definition (and aliases) have self argument - Make self in class have class as type - Remove args from GenericParent, it doesn't do anything. - Function arguments from python source is mutable. The application logic in class from AST is currently very convoluted. Could use a rewrite. * Test that py and mamba class can have generics TrueName now takes generic arguments from type. Now context also stores class generics from Mamba classes. It may very well be that this explains some weird behaviour we had around generics (in that they didn't work in most cases). In a future release we should test this further since I'm sure there are some cases we've missed. - Made note of #320 * Add optional to Name in generate Always print out generics in alphabetical order. This ensures that the output is easy to predict, we are not dependent on the whims of the iterator over the set, possibly preventing flaky tests. Make annotating output the default in tests as well. As we start to implement new features and debug the checking stage we should then automatically see old tests failing as well. * Make sorting of generics case-insensitive Add new description to README * Verify current unknown generic parent behaviour Currently, no check is done that a parent generic exists until the class is actually used. Perhaps in future this behaviour should be changed, however.
JSAbrahams
added a commit
that referenced
this issue
Jun 7, 2022
* Don't include Python init in GenericClassDef - Made note of #311 * Don't include Python init in GenericClassDef - Made note of #314 - Made note of #315 - Made note of #316 - Made note of #317 - Made note of #318 * Check that typed assign with expr does have type - Made note of #319 * Test functionality of class from AST - Type definition (and aliases) have self argument - Make self in class have class as type - Remove args from GenericParent, it doesn't do anything. - Function arguments from python source is mutable. The application logic in class from AST is currently very convoluted. Could use a rewrite. * Test that py and mamba class can have generics TrueName now takes generic arguments from type. Now context also stores class generics from Mamba classes. It may very well be that this explains some weird behaviour we had around generics (in that they didn't work in most cases). In a future release we should test this further since I'm sure there are some cases we've missed. - Made note of #320 * Add optional to Name in generate Always print out generics in alphabetical order. This ensures that the output is easy to predict, we are not dependent on the whims of the iterator over the set, possibly preventing flaky tests. Make annotating output the default in tests as well. As we start to implement new features and debug the checking stage we should then automatically see old tests failing as well. * Make sorting of generics case-insensitive Add new description to README * Verify current unknown generic parent behaviour Currently, no check is done that a parent generic exists until the class is actually used. Perhaps in future this behaviour should be changed, however.
JSAbrahams
added a commit
that referenced
this issue
Jun 8, 2022
* Don't include Python init in GenericClassDef - Made note of #311 * Don't include Python init in GenericClassDef - Made note of #314 - Made note of #315 - Made note of #316 - Made note of #317 - Made note of #318 * Check that typed assign with expr does have type - Made note of #319 * Test functionality of class from AST - Type definition (and aliases) have self argument - Make self in class have class as type - Remove args from GenericParent, it doesn't do anything. - Function arguments from python source is mutable. The application logic in class from AST is currently very convoluted. Could use a rewrite. * Test that py and mamba class can have generics TrueName now takes generic arguments from type. Now context also stores class generics from Mamba classes. It may very well be that this explains some weird behaviour we had around generics (in that they didn't work in most cases). In a future release we should test this further since I'm sure there are some cases we've missed. - Made note of #320 * Add optional to Name in generate Always print out generics in alphabetical order. This ensures that the output is easy to predict, we are not dependent on the whims of the iterator over the set, possibly preventing flaky tests. Make annotating output the default in tests as well. As we start to implement new features and debug the checking stage we should then automatically see old tests failing as well. * Make sorting of generics case-insensitive Add new description to README * Verify current unknown generic parent behaviour Currently, no check is done that a parent generic exists until the class is actually used. Perhaps in future this behaviour should be changed, however.
|
In general we need to think about how we call unsafe python code. |
|
Actually in the above, this is not valid Python code.
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Description of Bug
If class field not assigned to in Python constructor, it should be nullable.
How to Reproduce
If we have have class:
When building a Context, we have a class
MyClasswith fielda, which is an integer.Expected behavior
Field
ashould be typeOptional[Int], notInt.This is because it is never actually assigned to.
We could also just default to making it optional if it is never assigned to at the top level, of course.
Then afterwards we can do a check within the constructor to see if it is assigned to.
Additional context
This is tied to #311 , if we have implemented that logic, we can also use that to check whether class fields are actually assigned to.
If not, then we make it optional (if it is not already optional)
The text was updated successfully, but these errors were encountered: