lsst-epo / citizen-science-notebooks

A collection Jupyter notebooks that can be used to associate Rubin Science Platform data to a Zooniverse citizen science project.
3 stars 1 forks source link

Migrate some utils functionality from the variable_stars notebook into the utils package #82

Open beckynevin opened 8 months ago

beckynevin commented 8 months ago

@ericdrosas87 I'm not quite done polishing the functions that will need to move to utilities, so I could use some help if you wanted to glance over them and make suggestions for things I/we could change to bring them in line with what is currently in utils.py. My current working branch is variable_stars and the notebook is variable_stars_becky.ipynb. The relevant functions are towards the bottom of the notebook: get_cutout_image and make_calexp_fig and remove_figure. Of course, if you think these don't need to be in utils and can stay here, I'm also open to having that discussion.

This is all taking place in the variable_stars branch, and I think it would be best to migrate the utilities before we merge this branch with main.

ericdrosas87 commented 8 months ago

@beckynevin Yep I will take a look at the notebook and leave me notes here

ericdrosas87 commented 8 months ago

I know you said you weren't done cleaning up the functions and so this might already be on your radar, but the remove_figure() function should include the references that globally scoped in the notebook as input parameters and there should be a return value.

ericdrosas87 commented 8 months ago

Otherwise looks good

beckynevin commented 8 months ago

I know you said you weren't done cleaning up the functions and so this might already be on your radar, but the remove_figure() function should include the references that globally scoped in the notebook as input parameters and there should be a return value.

I'm working on this now - do you know what gc.collect() does? I know it's the garbage collector but do we need to assign it and then run some sort of additional removal like everything = gc.collect() del everything?

beckynevin commented 8 months ago

I know you said you weren't done cleaning up the functions and so this might already be on your radar, but the remove_figure() function should include the references that globally scoped in the notebook as input parameters and there should be a return value.

I'm also wondering, do we actually need a remove_figure() function? Can't we just do a plt.close() within the create_figure functions?

ericdrosas87 commented 8 months ago

I'm working on this now - do you know what gc.collect() does? I know it's the garbage collector but do we need to assign it and then run some sort of additional removal like everything = gc.collect() del everything?

It looks like the only thing gc.collect() returns is the number of objects in memory it cleaned up, so I don't think it's necessary to capture the return the value from gc.collect()

I'm also wondering, do we actually need a remove_figure() function? Can't we just do a plt.close() within the create_figure functions?

I'm honestly not sure, but my gut feel is that calling plt.close() should be sufficient. Calling .close() eventually causes the figure to be popped out of a list of figures which should be sufficient to be picked up when gc.collect() is called.

beckynevin commented 5 months ago

Moving a conversation @ericdrosas87 and I have been having on slack to here -

Essentially, the issue is that we have all of these plotting utilities that we'd like to migrate to utils.py. However, there's an experimental WCS option to many of these utilities. Our solution for now (suggested by @ericdrosas87 ):

It might be worth separating out as its own function. There’s no harm in leaving the function in the utils.py for now, but maybe include a print() statement in the function with a message stating that the function is experimental/deprecated for the time being just in case anyone comes across it and tries to use it