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

Color swatches from color maps that are suitable for grayscale printing #12

Merged
merged 26 commits into from
May 11, 2023

Conversation

sajidah-ahmed
Copy link
Member

No description provided.

README.md Outdated
import cosmoplots

color_list = cosmoplots.generate_hex_colors(5, 'viridis', show_swatch=True, ascending=True)
plt.savefig("/assets/color_test.png")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing a . in the path: -> ./assets/color_test.png.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for spotting. Done!

README.md Outdated
@@ -52,6 +52,23 @@ ax.plot(a)
plt.show()
```

## `generate_hex_colors`
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would move this to the end of the markdown file, so that the figure showing matplotlib vs cosmoplots is shown earlier.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've moved it to the bottom now

README.md Outdated
plt.savefig("./assets/color_test.png")

# Print color_list to retrieve the hex numbers
print(color_list)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When I run these lines in my terminal the last line prints out
['#fde725', '#5ec962', '#21918c', '#3b528b', '#440154']
Maybe add the output or remove the last line in order to be consistent.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, I've added the output

@gregordecristoforo gregordecristoforo requested a review from engeir May 11, 2023 09:15
@gregordecristoforo
Copy link
Member

@engeir Do you think this looks good?

@engeir
Copy link
Member

engeir commented May 11, 2023

I am happy with this. One thing that might be worth adjusting it the style of the docstrings, since elsewhere in the repo numpy style is used, it might be good to keep consistency.

I.e., instead of

"""...

Args:
    param: type
        desc

Return:
    type
        desc
"""
"""...

Parameters
----------
param: type
    desc

Returns
-------
type
    desc
"""

Generate plot of the desired color swatches. This is useful if you want to see what the swatches look like.
This function is used in generate_hex_colors() where the plot of the swatches will show by default if used in Jupyter Notebook.

Args:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since numpy style is used elsewhere, maybe change from

Args:

to

Parameters
----------

and similar for the rest.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've adjusted the style, how is it now?

@sajidah-ahmed sajidah-ahmed requested a review from engeir May 11, 2023 11:00

Returns
-------
color_swatch:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing return type

from typing import List


def make_color_swatch(color_list: list):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing return type

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Duplicate file

@sajidah-ahmed sajidah-ahmed merged commit e79fd81 into main May 11, 2023
@sajidah-ahmed sajidah-ahmed deleted the hex_colors branch May 11, 2023 11:35
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

Successfully merging this pull request may close these issues.

3 participants