-
-
Notifications
You must be signed in to change notification settings - Fork 285
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
Support defining methods of a function in multiple cells #177
Comments
I would like to track this feature in an actual issue, since at least I am really really missing this. Multiple dispatch and therefore multiple methods of the same function is a key feature of Julia. Often those methods are implemented in completely different contexts (for example |
(split into two issues: #178) |
It would be helpful if someone with an eye for edge cases (@karlwessel) would write the test cases for this before thinking up a solution. (Writing test cases is fairly easy, and they don't need to be perfect yet.) |
Strictly speaking, two cells
If all of these situations are allowed without a MultipleDefinitionsError (which is the easy solution), then a notebook could be ambiguous (ignoring the SECRET fact that the order in which cells are placed is used as a tie-breaker when two cells are mutually independent.) They need to be handled correctly. |
Is it possible to detect those cases during execution of a notebook by comparing the |
That's going in the right direction! (Although my 5th example would not get detected with that method) |
Well, maybe we don't have to catch all corner cases... |
Building on your counting methods idea: It's very nice if we can keep all of the syntax analysis in one place, before running the code, and then run cells in a precomputed order. Checking the number of defined methods after running cells (in the middle of a reactive run), and then taking that information back to do something special makes that tricky. But there are three reasons to do this kind of "dynamic" code analysis instead of static anaylsis:
The common hurdles in these three are:
Solution?We need to do some dynamic analysis, but we can try to keep it simple and isolated. The runner process will have three new APIs: do_signatures_overlap(signatures::Array{NTuple{Type}})::Bool
symbols_exported_by_module(name::Symbol)::Set{Symbol}
macroexpand(expr::Expr)::Expr to solve these three problems, respectively.
|
We can simplify the Although it is common to have user-defined types, I hope that it's uncommon for user defined aliases to cause this kind of trouble. So:
|
I guess the next step is me implementing those tests :-) |
Yes please! Although I think we will be going with a solution where half of those tests are broken :) |
This comment has been minimized.
This comment has been minimized.
Why is that? Is the method-counting version to complicated? |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Counting methods is not too complicated, but then there needs to be a feedback loop: too few methods were detected, so we error those cells manually, delete the methods, and then continue. Instead of: where each of those steps does not depend on the results of later steps |
It thought that was the process you described in your comment above. What is the simpler alternative? |
Hi! The complete process would be:
What don't work yet
A quick demo of the progressI have pushed my changes on this branch if you want to take a look at the implementation and compare the changes. If the methods implemented seems good to you I can open a draft PR to discuss more technical details of the implementation 🎇 |
(ramble) Hi @Pangoraw! Thanks so much for your contribution! I've sent you an email in case you would like to call! Your work looks promising, but motivated by @karlwessel (#177 (comment)) I think we should take a different direction: So instead, I think we should only attempt to catch super simple cases like I'm also not sure whether I like having the function signature info in its During my refactoring, I found that a bunch of tricky code can be eliminated! #536 |
The other solution is much harder to implement consistently - i.e. being able to detect multiple methods with the same argument types (instead of just detecting multiple functions with the same name) and deleting only the methods from one cell, while leaving the others.
All of this is possible within the scope of Pluto, but not something for the next couple of months :)
Originally posted by @fonsp in #109 (comment)
The text was updated successfully, but these errors were encountered: