Skip to content

Feature: Extend Segment Functionality #243

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

Open
wants to merge 11 commits into
base: main
Choose a base branch
from

Conversation

matt-humphrey
Copy link
Contributor

Summary

The purpose of this contribution is to begin extending the functionality of segments.
I've created a new segments.py which defines a function seg_group() which contains basic logic for allowing segments to contain groups of values.
I've made changes to validate.py to allow segments to handle None, and to handle seg_group().
Finally, I've created test_segments.py to test the different aspects of segments more thoroughly.

Related GitHub Issues and PRs

Checklist

@rich-iannone
Copy link
Member

This is looking really good! Initial thoughts:

  • add a docstring for seg_group() that also contains some examples (good model to follow is the starts_with() function in pointblank/column.py)
  • add seg_group to a new __all__ list in pointblank/segments.py
  • create a section and entry for seg_group() in docs/_quarto.yml; presumably there will be a few seg_*() helpers, so a Segmentation Helper Functions section seems like a good thing to do right now
  • perform very basic tests of SegmentGroup and seg_group() (mostly just for coverage purposes, but it's useful as a sort of 'snapshot')
  • later on, we may have to advertise the existence of the seg_*() helper functions in the docs of the validation methods (but that can wait until we have a good collection of them)

Once this is done, I think it'll be ready to merge! I think that additional seg_*() functions and addressing Windows test errors could be done in separate PRs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants