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

Package structure #72

Closed
fekad opened this issue Nov 5, 2019 · 5 comments · Fixed by #113
Closed

Package structure #72

fekad opened this issue Nov 5, 2019 · 5 comments · Fixed by #113
Assignees

Comments

@fekad
Copy link
Contributor

fekad commented Nov 5, 2019

  • All of the tests can be stored in a separate module (folder) to reduce clutter ins the package code. inspiration or like fastapi etc.
  • from the root folder, the images can be moved into an images folder and relative path can be used in the markdown.
  • in the future, maybe somebody will have time to write proper documentation for this package :)
optimade-python-tools
| - docs
| - images
| - optimade
    | - filterparser
        | -  grammars
    | - filtertransformers
    | - model
    | - server
| - tests
    | - filterparser
    | - filtertransformers
    | - model
    | - server
@fekad
Copy link
Contributor Author

fekad commented Nov 12, 2019

To keep the versions and the specs align (without matching the versions of the release, the pypi and the specification) the optimade repo can be added as a submodule:
git submodule add https://github.com/Materials-Consortia/OPTiMaDe.git specs

@fekad
Copy link
Contributor Author

fekad commented Nov 12, 2019

Is the support of multiple versions of the grammar is necessary? As far as I understand, all the releases planned to be tied to the version of the specification, isn't it?
To be honest I would use a single version with multiple variants (eg: elasticsearch, etc.) by implementing as class inherence instead of the factory method used by now.

@fekad fekad mentioned this issue Nov 12, 2019
6 tasks
@CasperWA
Copy link
Member

* All of the tests can be stored in a separate module (folder) to reduce clutter ins the package code. [inspiration](https://gist.github.com/tasdikrahman/2bdb3fb31136a3768fac#file-python_tests_dir_structure-md) or like [fastapi](https://github.com/tiangolo/fastapi) etc.

This I completely agree with, and have also been close to doing on a whim.

* from the root folder, the images can be moved into an `images` folder and relative path can be used in the markdown.

Also something that can easily just be done. I don't think anyone will have any objections if this was deftly included as a separate commit in a PR (or for best practices in a separate PR that only deals with reordering).

@CasperWA
Copy link
Member

To keep the versions and the specs align (without matching the versions of the release, the pypi and the specification) the optimade repo can be added as a submodule:
git submodule add https://github.com/Materials-Consortia/OPTiMaDe.git specs

Not a bad idea, but also not sure that this is necessary and may just clutter up the package a bit.

@CasperWA
Copy link
Member

Is the support of multiple versions of the grammar is necessary? As far as I understand, all the releases planned to be tied to the version of the specification, isn't it?

As per your comment here, I do not agree.
Since the spec supports different versions for the same back-end/repository, I think it prudent to keep all previous versions here as well.
Furthermore, if we always remove outdated versions and people are actually using this package to retrieve grammar, subclassing the filterparser or -transformer, we will have to make a new major release for each new grammar, since each release will not be backwards compatible and people will have to lock their versions used for the package. This may be a way we want to go, but I think it will be easier to simply just add instead of remove for this repo.
If one really wants to, then we can do a clean up for each new major spec release (i.e., when we go from 0.x to 1.x and so on).

Also, the comment that we need to keep the versioning of this package be equal to the spec if we keep in all the grammars doesn't sound right to me. Indeed I would expect this to be more important if we always only keep the newest grammar, since there will be no other way of knowing which grammar version you are getting from this package other than having the version match the spec.
If, on the other hand, we keep all grammar relating to a major spec version, we can get away with doing our own package versioning, and simply state that only the major version has to match with the spec, and that's it.

To be honest I would use a single version with multiple variants (eg: elasticsearch, etc.) by implementing as class inherence instead of the factory method used by now.

I am not completely sure what you mean by this, mostly how you would handle different versions through class inheritance.

@CasperWA CasperWA self-assigned this Jan 6, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants