search

pyapp-kit / magicgui

build GUIs from type annotations
https://pyapp-kit.github.io/magicgui/
MIT License
362 stars 49 forks source link

feat: add `Table.delete_row` method #610

Closed tlambert03 closed 11 months ago

tlambert03 commented 11 months ago

cc @jni

would appreciate your input on the API. I considered using Table.drop like the pandas API, but I thought it might be confusing that this one is in-place and that one is not. I'm also wary of the confusion between row indices and row headers (both of which could conceivably be integers), so I forced the usage of a kwarg.

codecov[bot] commented 11 months ago

Codecov Report

Attention: 3 lines in your changes are missing coverage. Please review.

Comparison is base (aa38630) 87.75% compared to head (6f41ae5) 87.73%.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #610 +/- ## ========================================== - Coverage 87.75% 87.73% -0.02% ========================================== Files 40 40 Lines 4705 4722 +17 ========================================== + Hits 4129 4143 +14 - Misses 576 579 +3 ``` | [Files](https://app.codecov.io/gh/pyapp-kit/magicgui/pull/610?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pyapp-kit) | Coverage Δ | | |---|---|---| | [src/magicgui/widgets/\_table.py](https://app.codecov.io/gh/pyapp-kit/magicgui/pull/610?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pyapp-kit#diff-c3JjL21hZ2ljZ3VpL3dpZGdldHMvX3RhYmxlLnB5) | `95.56% <82.35%> (-0.58%)` | :arrow_down: |

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

jni commented 11 months ago

Missing a couple of test coverage lines, but looks good to me. I basically agree with your assessment. I wouldn't look to the pandas API but the draft DataFrame API, which bizarrely has drop_columns but not drop_rows — perhaps they are leaking implementation details into their API choices? 😂 I also couldn't find an issue in their repo. But anyway, drop_columns appears to be in-place (returns Self), so maybe drop_rows would be a good analogous method name? And maybe we should make an issue in their repo, though I don't feel super strongly about it.

tlambert03 commented 11 months ago

Thanks!

Btw: I would not assume that it's in place because of -> Self. That means it returns "type of self" (not actual self)

See this type of usage for example: https://peps.python.org/pep-0673/#use-in-classmethod-signatures

jni commented 11 months ago

Fascinating! Thanks for the info!