grd349 / PBjam

A repo for our peak baggin code and tips on jam
MIT License
17 stars 6 forks source link

problem on save fig in star.peakbag.plot_echelle #194

Closed rgarcibus closed 4 years ago

rgarcibus commented 4 years ago

It seems that there is something missing when I try to save the plot of the echelon diagram in the following command:

star.peakbag.plot_echelle(pg).savefig('file.png')

The code says: /Library/Frameworks/Python.framework/Versions/3.7/lib/python3.7/site-packages/lightkurve/seismology/core.py:170: LightkurveWarning: You have passed both a numax and a frequency limit. The frequency limit will override the numax input. LightkurveWarning) Traceback (most recent call last): File "", line 1, in AttributeError: 'NoneType' object has no attribute 'savefig'

grd349 commented 4 years ago

@rgarcibus - Welcome :) . Thanks for reporting this!

@nielsenmb plot_echelle does not return a figure object so this is why it does not work. I guess it should. I'm going to fix this now and open a PR unless you are already on this.

nielsenmb commented 4 years ago

Yep, it's the missing fig return (I even wrote it should return this in the docstring)

Go ahead and fix it, I'm on my phone at the moment, so fixing code is challenging.

On Wed, 20 Nov 2019, 10:53 Guy Davies, notifications@github.com wrote:

@rgarcibus https://github.com/rgarcibus - Welcome :) . Thanks for reporting this!

@nielsenmb https://github.com/nielsenmb plot_echelle does not return a figure object so this is why it does not work. I guess it should. I'm going to fix this now and open a PR unless you are already on this.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/grd349/PBjam/issues/194?email_source=notifications&email_token=AEJWO35NNKCRG4PHWEMMEXLQUUJJLA5CNFSM4JPQT2H2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEERSD4Q#issuecomment-555950578, or unsubscribe https://github.com/notifications/unsubscribe-auth/AEJWO365RHR4PCW3HF4TR43QUUJJLANCNFSM4JPQT2HQ .

grd349 commented 4 years ago

Update - we use the lightkurve plot_echelle function and that doesn't give us access to the fig object. I guess this is why @nielsenmb you didn't return the fig object.

@ojhall94 I'm going to change this in lightkurve so that plot_echelle accepts an ax but if ax is None then create the fig and ax. I'll do this unless you know of a good reason not to :)

ojhall94 commented 4 years ago

You mean open a PR? I can't see a reason why it wouldn't be possible, but I'm also not sure why I didn't implement it in the first place. Weird!

grd349 commented 4 years ago

Yes a PR - just re-reading the contributing guidelines now :)

grd349 commented 4 years ago

PR is submitted - will probably want to wait to the next lk release before updating PBjam.

grd349 commented 4 years ago

Lightkurve 1.5 has been released and is pip installable. It has the update to the seismology.plot_echelle() function that we need. I will now deal with the fig return in our plot_echelle() and I update the requirements.

nielsenmb commented 4 years ago

That was fast!

On Thu, 21 Nov 2019, 10:44 Guy Davies, notifications@github.com wrote:

Lightkurve 1.5 has been released and is pip installable. It has the update to the seismology.plot_echelle() function that we need. I will now deal with the fig return in our plot_echelle() and I update the requirements.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/grd349/PBjam/issues/194?email_source=notifications&email_token=AEJWO356ZHMOBJJ7NSSOZSDQUZRATA5CNFSM4JPQT2H2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEEZZAPI#issuecomment-557027389, or unsubscribe https://github.com/notifications/unsubscribe-auth/AEJWO3Y42E5WVBXUAVAHGXLQUZRATANCNFSM4JPQT2HQ .

nielsenmb commented 4 years ago

close this?

grd349 commented 4 years ago

Indeed!