deepskies / DeepCMBsim

MIT License
2 stars 0 forks source link

plotting simple boiler plate plots for automatic outputs. #56

Closed samueldmcdermott closed 1 year ago

samueldmcdermott commented 1 year ago

@bnord -- right now there are functions that return arrays, which are easy to plot. I fear that making this too built in makes it less transparent (and much heavier, since we'd need to make matplotlib part of the dependency chain). Can you say more about what you want and how you'd want it to work?

bnord commented 1 year ago

I'd like to see some very basic diagnostic plots, like the ones that you've already made.

samueldmcdermott commented 1 year ago

I'm happy to save anything you think is illustrative, but can you say more about what specifically you'd like to see (plots of C_ells? flatmaps?) and how you'd like them to be stored (specific path, including a new folder if needed)?

cavestruz commented 1 year ago

@bnord - can you list plots in the a comment below that you'd like to see that @samueldmcdermott can check off that are not already in the example notebook? And put this into a list format? Otherwise, it looked like the example notebook has the main primary illustrative plots done and so we can close this.

bnord commented 1 year ago
  1. Is Matplotlib a heavy dependency?
bnord commented 1 year ago

suggested plot list

samueldmcdermott commented 1 year ago

changed the numbered list to a checklist

not sure what "heavy dependency" means. Are you asking "is matplotlib heavy?" (the answer is yes) or are you asking "is it critically important to do the stuff that our package does?" (the answer is no, it's only a small part of clplotting.py which could be deleted with no ramifications)

cavestruz commented 1 year ago

Just to follow up on Sam's note, having changed the numbered list to a checklist. With a checklist and the new linked branch to address the issue, GitHub provides an indicator of how close one is to being able to issue a PR (e.g. 3 out of 4 items completed). This helps with tracking feasibility and priorities if one, say, has 20min and wants to pick something out to check off.

On Fri, May 26, 2023, 12:55 samueldmcdermott @.***> wrote:

changed the numbered list to a checklist

not sure what "heavy dependency" means. Are you asking "is matplotlib heavy?" (the answer is yes) or are you asking "is it critically important to do the stuff that our package does?" (the answer is no, it's only a small part of clplotting.py which could be deleted with no ramifications)

— Reply to this email directly, view it on GitHub https://github.com/deepskies/simcmb/issues/56#issuecomment-1564666727, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABEVG65W3LP77GD4UR74EY3XIDOAHANCNFSM6AAAAAAXNAHR64 . You are receiving this because you commented.Message ID: @.***>

bnord commented 1 year ago

thanks, @cavestruz . I didn't know that about lists.

bnord commented 1 year ago

@samueldmcdermott what are best practices for including plotting libraries in science-related repositories?

samueldmcdermott commented 1 year ago

I'm really not sure -- because matplotlib is so heavy, and the functionality we use it for is so simple, I would vote to remove that function. But I'm not familiar with the best practice here. I'll defer to @humnaawan or @cavestruz on what to do

bnord commented 1 year ago

@voetberg @AeRabelais Could you chime in on this discussion? From perspective, how do you handle visualizations in code that you ship?

voetberg commented 1 year ago

@bnord

I think it's a fair assumption that if the end user wants to make visuals, they can make those visuals themselves and decide the specifics of the visuals.

If you're dead set on including visualization, you can always include the html file of an example notebook in documentation (reference most matplotlib example pages that include a coding example in a html)

bnord commented 1 year ago

I'm surprised about this, and I think I vehemently disagree, lol. But, I don't think I have enough experience to make a good argument.

@samueldmcdermott if you want to abandon or not go so far in making the viz, I'm fine with that. If I can figure out a good argument, I'll raise this issue again with more details.

samueldmcdermott commented 1 year ago

ok -- I'll delete the function power_spectrum_plot from clplotting.py, which is the only function that required matplotlib