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

joss review: confusing line in tutorial #330

Closed
wlav opened this issue Jul 10, 2017 · 3 comments
Closed

joss review: confusing line in tutorial #330

wlav opened this issue Jul 10, 2017 · 3 comments
Assignees
Labels

Comments

@wlav
Copy link

wlav commented Jul 10, 2017

This line had me puzzled:

filename = get_filepath('test.root')

b/c it's not clear that it is locating a file, as opposed to, say, creating or just constructing a temporary (unique) file name. Reading on, it's clear that it must to location, as the tree must exist.

Worth it to add a comment (there are several already anyway and this is a root_numpy API after all) and/or print 'filename' which makes it completely clear. Similarly, intree.Print() may be helpful to understand the branch selection part better.

@wlav
Copy link
Author

wlav commented Jul 10, 2017

After reading the API list, I now see that get_filepath is NOT part of the API. Does prove clearly, it was confusing me. :) Maybe another option then is to keep the 'testdata' part of the name to make obvious what is going on here.

@kratsg kratsg added the JOSS label Jul 10, 2017
@ndawe
Copy link
Member

ndawe commented Jul 11, 2017

Agreed. I'll use testdata.get_filepath() to make this clear.

@ndawe
Copy link
Member

ndawe commented Jul 18, 2017

Fixed in #336

@ndawe ndawe closed this as completed in 719c5f4 Jul 18, 2017
ndawe added a commit that referenced this issue Jul 18, 2017
FIX #330: use testdata.get_filepath()
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

3 participants