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

reiterate package structure #124

Open
filipsch opened this issue Aug 13, 2018 · 2 comments
Open

reiterate package structure #124

filipsch opened this issue Aug 13, 2018 · 2 comments

Comments

@filipsch
Copy link
Contributor

protowhat contains the functions that are shared between sqlwhat and shellwhat (and maybe pythonwhat at some point, see issue).

Protowhat contains a bunch of functionality:

  • State manipulation, the Ex() and F() chaining and >> syntax
  • The logic functions (check_correct() etc.)
  • Basic functions like has_chosen() and success_msg()

For these functions, protowhat makes total sense and should be kept. however, there is also AST-related functionality in protowhat, such as selectors, dispatchers, functions like check_field and check_node. These were pulled into protowhat becuase it was expected that both sqlwhat and shellwhat would use them. In reality however, to my knowledge there isn't a single shell exercise that has an SCT that uses the AST representation of a bash command.

Pulling the AST-related functionality back into sqlwhat would make the package easier to understand, easier to update and easier to test. Up for debate. cc @machow

@machow
Copy link
Contributor

machow commented Aug 14, 2018

I could see either way. My sense in terms of keeping in protowhat...

pros:

  • enforced separation of concerns (sql-specific functionality can't make its way into AST checking code)
  • easier to pull in to future libraries

cons:

  • requires two updates to deploy AST check features to DataCamp
  • have to download two packages to understand sqlwhat AST checks

I would lean toward keeping in protowhat, since we have 3 libraries doing AST checking: python, R (using parser data, but close enough), sql. In the future, it would be cool to distill what we've learned from these 3 AST approaches into one place.

For example, pythonwhat uses a visitor pattern, identical to protowhat, but with a twist that the visitors pull out conceptually useful info for SCTs (e.g. tries to infer what function was imported from from pandas import DataFrame as DF). This task is pretty similar to trying to figure out what the alias of a SQL query refers to, but I think it would be a huge headache for a person if pythonwhat and sqlwhat both implemented their own ways of doing this. It seems useful to have a clear sense for how protowhat could replace the pythonwhat specific code.

An example of protowhat being used with python is here: https://github.com/datacamp/protowhat/blob/master/example/example_python.ipynb

On the other hand, if AST based checks are going to become less important (e.g. they require too much development time), then I definitely could see moving the code into sqlwhat.

@filipsch
Copy link
Contributor Author

@machow thanks for your comments. I will not do any of this in the near future, I just wanted to put this issue out there for whoever takes over the maintenance of these packages.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants