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

Breaking up the python tools into seperable packages #255

Closed
shyamd opened this issue Apr 24, 2020 · 7 comments
Closed

Breaking up the python tools into seperable packages #255

shyamd opened this issue Apr 24, 2020 · 7 comments
Assignees
Labels
enhancement New feature or request priority/high Issue or PR with a consensus of high priority

Comments

@shyamd
Copy link
Contributor

shyamd commented Apr 24, 2020

The optimade-python-tools repo is now rather complex with several components. I suggest we break this up. One option is to make this into separate repos. Another is to make this into namespace packages, which I like lately. Namespace packages allow all of this to be developed in this repo, but deployed as independent packages.

I'm suggesting the following packages:
optimade : meta-package for all optimade subpackages
optimade-core: optimade.models, optimade.grammar,optimade.filterparser, optimade.filtertransformers
optimade-server: optimade.server
optimade-validator: optimade.validator
optimade-client: optimade.adapters + a future CLI or GUI?

@ml-evs
Copy link
Member

ml-evs commented Apr 24, 2020

I think the filter transformers should also be moved out of core as they're often using functionality from the backend to construct them (mongo is the exception).

Apart from that, this proposed package structure looks reasonable to me.

@CasperWA
Copy link
Member

I agree about the transformers, it should probably be moved to optimade-server. However that said, it sort of lies in between the two. A tricky one, which I will be able to vote for both spaces in the end.
Already I feel some arguments tugging me back to core... 🙄

Concerning the client, I think we should still hold on the fact that this should only be a repository for tools, i.e., we should avoid having an actual client in the repo. The adapter is a tool for a client (as I see it), so it's fine 😅
I'm even keen to separate out the server, since it's not technically a "tool", but rather an actual implementation. However, for this it's a bit more iffy again, since other implementations indeed use the server here as a tool for the basis of their own implementation ... So I don't know about that, but would like to see it go eventually.

Other than those thoughts, I'm keen to start splitting this repo up in this way 👍

@fekad
Copy link
Contributor

fekad commented May 1, 2020

Hi, I would agree to move transformers into optimade-server because it's implementation is depends on the implementation of the server.
I would maybe merge optimade.grammar and optimade.filterparser into something like optimade.filter

@fekad
Copy link
Contributor

fekad commented May 3, 2020

It might be a good time to add some extra folders like:

  • docs: It would be nice to have documentation which can be built/deployed to GitHub pages. As far as I know, it was already discussed on slack.
  • examples: could contain a demo server and some notebooks to demonstrate how the query/adaptors/CLI works

@shyamd
Copy link
Contributor Author

shyamd commented May 4, 2020

All makes sense. Give me till tomorrow afternoon and I'll have a PR for all of this.

@CasperWA
Copy link
Member

CasperWA commented May 4, 2020

  • docs: It would be nice to have documentation which can be built/deployed to GitHub pages. As far as I know, it was already discussed on slack.
  • examples: could contain a demo server and some notebooks to demonstrate how the query/adaptors/CLI works

I would probably consider the examples folder here a part of the docs, but it is a very nice idea to include.

@CasperWA CasperWA added enhancement New feature or request priority/medium Issue or PR with a consensus of medium priority labels May 6, 2020
@ml-evs ml-evs added priority/high Issue or PR with a consensus of high priority and removed priority/medium Issue or PR with a consensus of medium priority labels Jun 5, 2020
@ml-evs
Copy link
Member

ml-evs commented Jun 11, 2020

Am I safe to close this? It seems we have decided to no split the package up for now due to the technical overhead, and I'm not sure there's anything left to discuss. Feel free to re-open if you disagree.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request priority/high Issue or PR with a consensus of high priority
Projects
None yet
Development

No branches or pull requests

4 participants