Skip to content
This repository has been archived by the owner on Jan 7, 2023. It is now read-only.
This repository has been archived by the owner on Jan 7, 2023. It is now read-only.

Use "entries" instead of "N" as arg name #23

Closed
ndawe opened this issue Dec 12, 2012 · 15 comments
Closed

Use "entries" instead of "N" as arg name #23

ndawe opened this issue Dec 12, 2012 · 15 comments

Comments

@ndawe
Copy link
Member

ndawe commented Dec 12, 2012

def tree2rec(tree, branches=None, entries=None, offset=0,                             
        include_weight=False,                                                   
        weight_name='weight',                                                   
        weight_dtype='f4'):
def tree2array(tree, branches=None, entries=None, offset=0,                           
        include_weight=False,                                                   
        weight_name='weight',                                                   
        weight_dtype='f4'):

@piti118 what do you think?

@piti118
Copy link
Contributor

piti118 commented Dec 12, 2012

What is numpy using for their slicing methods? We better be consistent with them.

@piti118 piti118 closed this as completed Dec 12, 2012
@piti118 piti118 reopened this Dec 12, 2012
@piti118
Copy link
Contributor

piti118 commented Dec 12, 2012

Actually I think you are right we'd better be consistent with root and call it entries.

You will need to change test method too.

@ndawe
Copy link
Member Author

ndawe commented Dec 12, 2012

ROOT uses nentries and firstentry

@piti118
Copy link
Contributor

piti118 commented Dec 12, 2012

And also do the same thing with root2rec and root2array

@ndawe
Copy link
Member Author

ndawe commented Dec 12, 2012

right

@piti118
Copy link
Contributor

piti118 commented Dec 12, 2012

nentries and firstentry seems overkill

@ndawe
Copy link
Member Author

ndawe commented Dec 12, 2012

I agree. I prefer entries and offset

@ndawe
Copy link
Member Author

ndawe commented Dec 12, 2012

Although offset isn't clear if it's inclusive or exclusive. firstentry is more clear.

@ndawe
Copy link
Member Author

ndawe commented Dec 12, 2012

well... maybe it is clear...

@piti118
Copy link
Contributor

piti118 commented Dec 12, 2012

I'm trying to find an equivalent in numpy they called number of bins in histograms bins

@piti118
Copy link
Contributor

piti118 commented Dec 12, 2012

It'd be hard to fix it again let's be thorough

@piti118
Copy link
Contributor

piti118 commented Dec 12, 2012

numpy use offset throughout http://docs.scipy.org/doc/numpy/reference/generated/numpy.ndarray.html

So, I think the best choice is offset and entries.

@ndawe
Copy link
Member Author

ndawe commented Dec 12, 2012

Sounds good.

@piti118
Copy link
Contributor

piti118 commented Dec 13, 2012

Do you want to implement this change?

@ndawe
Copy link
Member Author

ndawe commented Dec 13, 2012

Sure, I'll submit a PR now.

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

No branches or pull requests

2 participants