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

cols and df_view_col passed to downstream functions #182

Merged
merged 19 commits into from
Jan 19, 2024

Conversation

smitkadvani
Copy link
Contributor

@smitkadvani smitkadvani commented Jan 16, 2024

  • cols and df_view_cols passed to downstream functions.
  • Running following example returns True and doesn't break
import bioframe as bf
import pandas as pd

cols = ['chrom', 'start', 'end']
data = [
    ('chr1', 1000000, 1250000),
    ('chr1', 1500000, 1750000),
    ('chr2', 1000000, 1300000),
    ('chr2', 2000000, 2300000)
]
df = pd.DataFrame(data, columns=cols)

# Create a sample DataFrame
view_cols = ['CHROM', 'START', 'END']
data = [
    ('chr1', 1000000, 1250000),
    ('chr1', 1500000, 1750000),
    ('chr2', 1000000, 1300000),
    ('chr2', 2000000, 2300000)
]
view_df = pd.DataFrame(data, columns=view_cols)

bf.is_contained(df, view_df, cols=cols, view_cols=view_cols)

@gfudenberg
Copy link
Member

thanks for the fix @smitkadvani !

Does the above still work if the view_df has a column "name", e.g.

columns=["chrom", "start", "end", "name"],

It would be great to modify the above code example and include it as a test

def test_is_contained():

(or modify one of the existing tests to have a df & view_df with non-conventional column names)

We should also add the case of nonstandard df_view_col and view_name_col (e.g. view_name_col="NAME" and df_view_col="VIEWREGION") to tests.

For consistency it also looks like next few functions in checks could use view_cols to support non-default names for those as well (is_covering, is_tiling, and is_sorted)-- this would be great to address in same PR!

Copy link
Member

@gfudenberg gfudenberg left a comment

Choose a reason for hiding this comment

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

great!

@gfudenberg gfudenberg merged commit bf0416a into open2c:main Jan 19, 2024
6 checks passed
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