Validate docstring examples#384
Conversation
github-actions
bot
added
part:actor
Marenz
force-pushed
the
validate_docstring_examples
branch
3 times, most recently
from
eeb75ed to
abf9358
Compare
Marenz
force-pushed
the
validate_docstring_examples
branch
from
abf9358 to
961b8f6
Compare
|
@leandro-lucarella-frequenz I tried having the script in another file/dir than |
leandro-lucarella-frequenz
changed the title
leandro-lucarella-frequenz
left a comment
leandro-lucarella-frequenz
left a comment
There was a problem hiding this comment.
Just a few minor things.
src/frequenz/sdk/actor/power_distributing/power_distributing.py
Outdated
Show resolved
Hide resolved
| # pylint: disable=import-error | ||
| import polars as pl |
There was a problem hiding this comment.
There is still the question about what to do about this. @matthias-wende-frequenz do you think we can avoid using polars in the example? Or is it very important for polars to be used here? I don't think it is terrible to have a pylint: disable=xxx in an example, but if we can avoid it, much better.
There was a problem hiding this comment.
If we remove polars we can remove the whole example. So the discussion is more about do we need the example and my feeling is, that removing an example to make a tool work is not the best practice.
There was a problem hiding this comment.
OK, I see, although the discussion is not if we need the example or not. I think we all agree we want the example, I just thought maybe it was possible to do some example/demonstration just using numpy that was equivalent (since we already have it as a dependency). This is just exposing my ignorance about pandas/numpy if I'm not making any sense, but I didn't wanted to remove examples to adapt to the tool :)
In any case I think we need to strike a balance, because also adding obscure # pylint: disable=import-error (that could be carried out blinding by users) is also not great.
Adding support to specify dependencies in code examples is also quite a bit of work and will make running this examples much slower. An alternative could be to offload examples needing external libraries to examples/, where it should be easier to specify dependencies for example, but I agree in this case that defeats the whole purpose of having a quick small example embedded.
In any case, I agree that for now if there is no consensus about this, the best approach is to leave the # pylint: disable=import-error, but I think the goal should be to somehow remove it in the future. I will create a discussion about this so it's not forgotten.
There was a problem hiding this comment.
OK, I just moved the topic here for now:
Marenz
force-pushed
the
validate_docstring_examples
branch
from
961b8f6 to
6c94354
Compare
leandro-lucarella-frequenz
left a comment
leandro-lucarella-frequenz
left a comment
There was a problem hiding this comment.
Found one error in the examples undetected by pylint.
Marenz
force-pushed
the
validate_docstring_examples
branch
from
6c94354 to
ce34f7a
Compare
github-merge-queue
bot
removed this pull request from the merge queue due to failed status checks
Signed-off-by: Mathias L. Baumann <[email protected]>
Signed-off-by: Mathias L. Baumann <[email protected]>
Marenz
force-pushed
the
validate_docstring_examples
branch
from
ce34f7a to
b25272d
Compare
This commit adds a module with an utility function to be able to easily collect and lint code examples in docstrings. It also add an optional dependency to easily pull the dependencies needed to do this linting and ignores any optional dependency starting with `extra-` in the tests checking if new repo types were added, as optional dependencies starting with `extra-` are not really repository types. This is based on the work on the SDK: frequenz-floss/frequenz-sdk-python#384 Signed-off-by: Leandro Lucarella <[email protected]>
This commit adds a module with an utility function to be able to easily collect and lint code examples in docstrings. It also add an optional dependency to easily pull the dependencies needed to do this linting and ignores any optional dependency starting with `extra-` in the tests checking if new repo types were added, as optional dependencies starting with `extra-` are not really repository types. This is based on the work on the SDK: frequenz-floss/frequenz-sdk-python#384 Signed-off-by: Leandro Lucarella <[email protected]>
This commit adds a module with an utility function to be able to easily collect and lint code examples in docstrings. It also add an optional dependency to easily pull the dependencies needed to do this linting and ignores any optional dependency starting with `extra-` in the tests checking if new repo types were added, as optional dependencies starting with `extra-` are not really repository types. This is based on the work on the SDK: frequenz-floss/frequenz-sdk-python#384 Signed-off-by: Leandro Lucarella <[email protected]>
3 participants
fixes #81