implement projected dependencies (fix #436) #506
Conversation
| -> Experiment | ||
| --- | ||
| (first) ->[unique, nullable] User | ||
| -> [unique, nullable] User.proj(first="username") |
There was a problem hiding this comment.
Actually, given that we are keeping forward compatibility, we need to ensure that old syntax still works. Please add back the old syntax to the test.
There was a problem hiding this comment.
I did leave some dependencies in the old syntax. IndexRich is a new test, so I used the new syntax. Old tests use old syntax.
| definition = """ # a synapse within the slice | ||
| -> Cell | ||
| (input)-> Cell(cell) | ||
| -> Cell.proj(input="cell") |
There was a problem hiding this comment.
While this table uses the new syntax, many existing tests use the old syntax. They don't show in the diff because they remained unchanged.
| definition = """ | ||
| # a synapse within the slice | ||
| (pre_slice, pre_cell) -> Cell(slice, cell) | ||
| -> Cell.proj(pre_slice="slice", pre_cell="cell") |
There was a problem hiding this comment.
The current tests cover both syntaxes.
| raise DataJointError('Parsing error in line "%s". %s.' % (line, err)) | ||
| except pp.ParseException: | ||
| try: | ||
| result = foreign_key_parser_old.parseString(line) |
There was a problem hiding this comment.
I think it would be best to throw a warning of some kind to indicate that they should update their syntax.
There was a problem hiding this comment.
Perhaps in a future release after the complete documentation is in place. How about we make the warning in release 0.12 and raise an exception in 0.13?
There was a problem hiding this comment.
Ok I'm cool with waiting to throw a warning till 0.12. Let's see about turning it into an exception. It may just be more appropriate to make a release of 1.0.0 where we switch on exception for all warnings...
There was a problem hiding this comment.
I think the switch from 0.. to 1.0.0 should be gentle, with no dramatic changes for a smooth roll out. We can make 0.13.0 the release where all the obsolete features are deprecated.
There was a problem hiding this comment.
Ok let's figure out the timing but yeah I'm fine making 0.x to 1.0 switch smooth.
|
Must say that the review could be more thorough but will perform thorough follow up testing. |
3 participants
describe