sphinx-needs
sphinx-needs copied to clipboard
use a class for 'need info' instead of a dictionary
the 'need info' should be a class rather than a dictionary
- attributes are strongly-typed
- methods can be added to the class for encapsulation
for example, "trimmed_title" is an attribute in the need info dict which is added during creation in the api.add_need
method. This could instead be a method on a NeedInfo
class, which would improve encapsulation.
I've created a proof of concept of this approach using dataclasses based on the 'todo extension' example, but you could also use plain classes - https://github.com/danieleades/sphinx-todo/blob/1d17716a827b8c7cbcb12b318a471658487b5d7d/src/sphinx_todo/init.py#L50
If this was of interest I would need some help to pull it together. The 'needs info' is created in a couple of different places in the codebase, and different attributes are added at different times (it's pretty complex right now). I could perhaps make a start on this, but would need some support.
This doesn't need to be a breaking change, as the construction of needs could be entirely internal to the library, however it would ultimately be tidier to expose the class as part of the external api, and have 'add_need' accept the class, rather than the mass of args that it currently takes.
thoughts?
Puhh, that would be a lot of work to do. From the technical point of view, this makes totally sense. But I'm afraid that there are so many pieces in the code, which are working with this object, that really every directive/feature will be affected. And also the review will be huge, as this review can't be done step by step.
Not sure if I can help here much, as my time is limited.
I would also wait 1-2 months with such a refactoring, as we are just preparing release 1.0, which will introduce some new features and therefore will have some/many follow-up bug fixes and code changes.
To be honest, I'm not sure if such a change is worth the effort.
I have a couple of long-haul flights coming up, so I might have some spare time on my hands ;)
I'll see how I go.
Converting dictionary access calls to class attribute calls will be largely mechanical. However the code changes around 1.0 are very likely to lead to churn and difficulty rebasing, i agree with you there.
Please go like this. With #611 I would have the demand, to have an API to automatically translate a need in a pure dict(str, str). This would easly possible with a class and a methode for.
How can I help?
Please go like this. With #611 I would have the demand, to have an API to automatically translate a need in a pure dict(str, str). This would easly possible with a class and a methode for.
How can I help?
i think an interim solution is to use a TypedDict
to provide typings without any API changes. Once that's done, migrating from TypedDict
to a class or dataclass is relatively trivial