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

_data_dict update method #84

Open
cduff4464 opened this issue Jul 20, 2016 · 12 comments
Open

_data_dict update method #84

cduff4464 opened this issue Jul 20, 2016 · 12 comments

Comments

@cduff4464
Copy link

Hey I am using some of the xray-vision library for an interface that I am making for XPD/28-ID beam line. I need the cross section 2-D view to be able to update its _data_dict so that it can take in more images while running. I am willing to make a PR for this, but I was wondering if such a method already existed in the code.

@CJ-Wright
Copy link
Contributor

You might look at this and this

@cduff4464
Copy link
Author

Tried using these, they require something called a corner list, that I am not sure how to make. I found a work around however, that lets you access the dictionary directly from where you call the cross-section 2D view class and go from there thanks though.

@CJ-Wright
Copy link
Contributor

Why does it require a corner list, isn't that optional?

@cduff4464
Copy link
Author

I tried sending in none a few times, and it refused to update. Then I tried sending in a list with some random values to no avail. Nothing made the dictionary update.

@CJ-Wright
Copy link
Contributor

Hmm that's odd.
@tacaswell @ericdill Thoughts? It seems that self.find_corners might not exist for the Stack classes
Also it seems that the docstring does not match the method signature.

@cduff4464 Do you have this workaround in your branch? If not when you get a chance can you put it up? I would be very interested in looking at it.

@tacaswell
Copy link
Member

That code looks wrong.

@tacaswell tacaswell reopened this Jul 21, 2016
@cduff4464
Copy link
Author

cduff4464 commented Jul 22, 2016

Yeah the workaround that I made is currently in my branch https://github.com/cduff4464/xray-vision/blob/master/xray_vision/qt_widgets/__init__.py under the new_data method in the CrossSectionMainWindow class @CJ-Wright

I just made it add the new data to the dictionary directly, I also made key_list one of the classes variable that way I could just append the new key name to it for further use in the code. The last two lines in the method just re-initialize the widgets associated with the image changing.

@CJ-Wright
Copy link
Contributor

CJ-Wright commented Jul 22, 2016

@cduff4464 Great thank you very much! Just FYI usually when you fork you create your own branch with a short name of the feature, that way the PR isn't master to master.

Would it be possible to add your new_data method to the messenger class? It seems that all your code references it with the exception of the first line.

why are there two versions of the main_window attribute, one protected and the other not?

Also why is there a self.key_list sitting in the middle of the __init__?

@cduff4464
Copy link
Author

That is a typo on the unprotected one. I just fixed it. The self.key_list is another type, I had it set to the key_list that gets sent int that way it could be referenced in the add_data method later on. I assume it would work just fine from the messenger class. I'll make a branch and make a pull request for it.

@cduff4464
Copy link
Author

cduff4464 commented Jul 22, 2016

Also really quick which messenger class did you mean in particular. I just want to make sure I add it to the right one.
@CJ-Wright

@CJ-Wright
Copy link
Contributor

CrossSection2DMessenger I think. Although if you find that the 1D stack also lacks an add data method you might add it to a more abstract messenger. The idea is that the issue you solved is in the messenger, as evinced by all the lines in the new method calling or setting attributes of the messenger. Thus you should make the changes in the messenger.

@cduff4464
Copy link
Author

cduff4464 commented Jul 25, 2016

I made the PR with the fix in it.
@CJ-Wright

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

No branches or pull requests

3 participants