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

Propagate changes to the line index into the GUI code #791

Merged
merged 3 commits into from
Oct 6, 2017

Conversation

yeganer
Copy link
Contributor

@yeganer yeganer commented Oct 6, 2017

This should fix #790

@wkerzendorf
Copy link
Member

@yeganer thank you for your help. I think it is easier if we simply reindex within the gui to line_id as this is specifically the index that is being saved.

@yeganer
Copy link
Contributor Author

yeganer commented Oct 6, 2017

I thought about that. Both indices have their benefits in my opinion. Using atom_data, ion_number, ... makes several filtering operations a lot easier and straight forward.

@unoebauer
Copy link
Contributor

Hi @yeganer - just tested these changes with tardis_example:

config = tardis.yaml_load("tardis_example_new.yml")
mdl = tardis.run_tardis(config)
from tardis.gui import interface
interface.show(mdl)

After the gui fires up, I try to show the line info in some wavelength window and it still crashes with the same error.

@unoebauer
Copy link
Contributor

The minimal change to fix this error is (and I tested this!) to do

self.lines = lines.reset_index().set_index("line_id")

in tardis/analysis.py on line 27 and

self.lines_data = lines_data.reset_index().set_index('line_id')

in tardis/gui/widgets.py line 962.

@yeganer
Copy link
Contributor Author

yeganer commented Oct 6, 2017

@unoebauer This is very strange because it works on my end. If the error is the same, then it looks like the changes made by this PR were not applied.

@unoebauer
Copy link
Contributor

@yeganer - turns out that I still had pandas 0.19.1 installed. Updated to 0.20 (as requested in the conda env file) and everything works.

@wkerzendorf wkerzendorf merged commit 696910f into tardis-sn:master Oct 6, 2017
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.

Last interaction info missing
3 participants