Fix simple errors reported by docstub (I)#7853
Conversation
This will also be correctly formatted by sphinx.
See [1] for more context on "extra info" and the allowed format. [1] https://github.com/scientific-python/docstub/blob/main/docs/typing_syntax.md
In the case of `denoisy_wavelet` and and active contour the literal list would have gotten quite long, so I'm opting for "str" for now. Later we could provide a more precise type as an inline annotation if we wanted.
| Offset added to the footprint center point. Shift is bounded to the | ||
| footprint sizes (center must be inside the given footprint). | ||
| p0, p1 : float in [0, ..., 1] | ||
| p0, p1 : float, optional, in interval [0, 1] |
There was a problem hiding this comment.
Would be nice to be able to spell this as float in interval [0, 1], optional
There was a problem hiding this comment.
Hmm, with some modification of docstub's grammar this would be totally doable. We'd have to come up with a consistent expectation. E.g.
interval: qualname "in interval" INTERVAL_BRACE number "," number INTERVAL_BRACE
INTERVAL_BRACE: "[" | "]" | "(" | ")"
might work. We could also support "in range". What I don't like is that we have to hardcode the "in interval" string. Otherwise it could be potentially confused with other expressions in the grammar.
There's a cost to doing this. Python's type system doesn't support this (yet, as far as I know). Docstub could either drop it or encode it with typing.Annotated.
Maybe another solution might make this more readable? E.g. the docstub expects the following form
name : annotation, optional, extra_info
Maybe we could allow switching "optional" and "extra_info" around. That way one could write
p0, p1 : float, in interval [0, 1], optional
Doesn't feel like an immediate priority though. Curious what you think.
There was a problem hiding this comment.
Created scientific-python/docstub#65 to continue / preserve this discussion.
There was a problem hiding this comment.
Yes, as long as optional is last, I think that reads fine. Optional is not super helpful, since it's already encoded in the function signature.
Could also use float in [0, 1]
Co-authored-by: Stefan van der Walt <stefanv@berkeley.edu>
Neither bigger nor smaller! I find this size to be the maximum manageable one for an effective review.
Thanks for the reminder, the commit breakdown is indeed helpful. |
Description
There will be follow up PRs but I'd like to keep them at a manageable review size. Let me know if follow PRs should be bigger or smaller.
Reviewing commit by commit isn't a bad idea, since that groups similar fixes together.
Checklist
./doc/examplesfor new featuresRelease note
For maintainers and optionally contributors, please refer to the instructions on how to document this PR for the release notes.