#106 suggestion for private variable names #110
Conversation
| @@ -340,7 +340,7 @@ def from_hdf5(cls, fname=None): | |||
| synpp_refs = None | |||
|
|
|||
| if 'ion_cx_data' in h5_datasets and 'ion_cx_data' in h5_datasets: | |||
| ion_cx_data = read_ion_cx_data(fname) | |||
| ion_cx_data = None | |||
There was a problem hiding this comment.
what happened there? Is this associated with your new bound-free code?
There was a problem hiding this comment.
if you don't need that anymore we could take it out entirely.
There was a problem hiding this comment.
Yes, this is associated with the new bound-free code because the data structure has changed. I can remove this from this PR.
There was a problem hiding this comment.
in the end currently whatever happens ion_cx_dta=None which is a little strange. But your the BF expert and if that is needed by you later leave it in.I think I'm happy either or, if everyone's happy just merge
There was a problem hiding this comment.
Should I remove the changes?
There was a problem hiding this comment.
@mklauser I think it's up to you whether you want these changes in there just now or not. Either is fine. Feel free to merge this as it is or remove those changes and then merge - whichever you prefer.
I think it looks good! Thanks for sorting this one out.
|
This works - I have tried with this version and that Pandas warning goes away. I propose we merge and then close this (and terminate pull request 106). |
|
looks good - apart from the comment I'm happy to merge - let's open an issue for including testing atomic.py. other comments @ssim? |
#106 suggestion for private variable names
For issue #106.