-
Notifications
You must be signed in to change notification settings - Fork 23
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
Adding in Y24 #228
Adding in Y24 #228
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #228 +/- ##
=======================================
Coverage 99.89% 99.89%
=======================================
Files 8 8
Lines 919 935 +16
=======================================
+ Hits 918 934 +16
Misses 1 1 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good!
Just one comment, as we discussed before, you need to make sure all models have a different color. I often use matplotlib's tab20 color map (https://matplotlib.org/stable/users/explain/colors/colormaps.html) if I need more than 10 colors.
dust_extinction/grain_models.py
Outdated
|
||
class Y24(BaseExtGrainModel): | ||
r""" | ||
Ysard et al. (2024) Grain Model |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Might be worth mentioning that this is THEMIS 2.0, to avoid confusion in the future.
Can you share how you do this? |
Yes, first you create a color map:
Then, you can use that map in your for loop, and loop through the 20 colors in that colormap with the modulo operator:
I believe "tab10" is the default that matplotlib uses if you do not specify the color, but if you have more than 10 models, you will run out of colors. "tab20" gives you 20 colors, so will last a little longer, at least until we have 20 extinction models ;) |
Updated based on comments. |
Add in Ysard et al. (2024) grain model curve.
Closes #223.