Refactor backend classes to facilitate adding more #347
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Owner
|
This is awesome - thanks! I'll try to do a proper review ASAP. |
Author
|
Should I rebase this? |
Owner
|
@DavidMStraub: Sorry about dropping the ball on this!! Yes - that would be amazing if you're up for it. Thanks for your patience. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Hi,
I wanted to add one or two additional backends - specifically
numpy.savezand possibly SQLite (or generic SQLAlchemy). However when I looked at the code, the existingBackendandHDFBackenddid not inherit from a common base class and it was not very transparent in my view which methods were shared/generic and which were implementation dependent. So in this PR, I just reshuffled the code - without changing anything about the API - such that implementation-independent methods are relegated to an abstract base class. Furthermore, an abstract class for file-based backends is added. In this way, it would be straightforward for me to add anumpy.savezbackend as a next step.Sorry I did not discuss this on the emcee-useres mailing list before opening the PR - I made the mistake to read the contributing guidelines only when it was done - but I hope you find it useful nevertheless.