Skip to content
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

Added Functionality ot Pivot_longer #176

Merged
merged 11 commits into from
Apr 14, 2022
Merged

Conversation

OscarDeGar
Copy link
Collaborator

Changes:

  • Selection helpers work stand alone with pivot_longer
  • Names_sep can now take an int as position for separation.
  • Names_pattern added to allow for capture groups/regex split via a pattern.

@codecov
Copy link

codecov bot commented Apr 9, 2022

Codecov Report

Merging #176 (41813a6) into master (b77b3b2) will increase coverage by 0.15%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master     #176      +/-   ##
==========================================
+ Coverage   76.59%   76.75%   +0.15%     
==========================================
  Files          85       85              
  Lines        6439     6482      +43     
==========================================
+ Hits         4932     4975      +43     
  Misses       1507     1507              
Impacted Files Coverage Δ
grama/dfply/select.py 93.10% <ø> (ø)
grama/tran_pivot.py 100.00% <100.00%> (ø)
grama/fit_synonyms.py 94.44% <0.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b77b3b2...41813a6. Read the comment docs.

Copy link
Owner

@zdelrosario zdelrosario left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome work so far! Made some minor comments about the index behavior of names_sep and unittesting.

stang = data.df_stang_wide
long = gr.tran_pivot_longer(
stang,
names_pattern="(E_|mu_)(\\d+)",
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A good test to implement, but the pattern should be names_pattern="(E|mu)_(\\d+)".

self.assertTrue(result)


def test_pivot_longer_names_sep_and_pattern(self):
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice!

stang = data.df_stang_wide
long = gr.tran_pivot_longer(
stang,
names_sep=-3,
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The sep argument in tran_separate() is provided as a list; this is so one can specify multiple separation points for multiple into entries. The names_sep argument should provide similar functionality with names_to; i.e. one should be able to provide arguments where len(names_to) - 1 == len(names_sep).

grama/tran_pivot.py Show resolved Hide resolved
Oscar De La Garza added 4 commits April 13, 2022 15:47
-Fixed test case for pivot_longer, line203
-Positional names_sep now takes a list of x length
-CodeCov hit fixed
Copy link
Owner

@zdelrosario zdelrosario left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Small edits to rm experimental code, but overall looks good!

return longer


def position_split(names, names_sep):
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe outdated code?

stang = data.df_stang_wide
long = gr.tran_pivot_longer(
stang,
columns = gr.matches("\\d+"),
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LOVE IT

stang = data.df_stang_wide
long = gr.tran_pivot_longer(
stang,
names_pattern="(E|mu)_(\\d+)",
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AMAZING

@zdelrosario zdelrosario merged commit cfea972 into zdelrosario:master Apr 14, 2022
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