-
Notifications
You must be signed in to change notification settings - Fork 133
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
Factor out annotation-related functionality into a separate module #439
Factor out annotation-related functionality into a separate module #439
Conversation
@zhiltsov-max This PR is incomplete (I haven't updated the examples in the documentation yet). Let me know if you agree with this refactoring and if you do, I'll finish it. |
Thought of it. It does not really matter until we have stable and documented API, but this breaks the library API. |
I could add compatibility exports if that's your concern. Although I kinda figured that with the versioning changes we're pretty much breaking all compatibility anyway. :P |
So what should I do? Should I add the compatibility exports? |
Let's put off the PR till we introduce the stable API after the versioning release. |
I don't think that's a good course of action; it would be much harder to bring the PR up-to-date at that point (which could be long into the future) than it would be to just add compatibility exports now and then remove them later. Plus, by merging this now we can improve the navigability of |
Ok, then add compatibility exports, and it is ready. |
Annotations are only tangentially related to extractors, so I don't think they belong in extractor.py. `extractor.py` is also a large module, so removing a chunk of it makes it easier to navigate.
OK, done and updated the documentation. |
Summary
Annotations are only tangentially related to extractors, so I don't think they belong in
extractor.py
.extractor.py
is also a large module, so removing a chunk of it makes it easier to navigate.How to test
Checklist
develop
branchLicense
Feel free to contact the maintainers if that's a concern.