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

Support alternate color palette #106

Merged
merged 8 commits into from
Aug 4, 2023
Merged

Support alternate color palette #106

merged 8 commits into from
Aug 4, 2023

Conversation

josephmckinsey
Copy link
Contributor

@josephmckinsey josephmckinsey commented Mar 24, 2023

Adds an alternate color palette:

  1. You can set an environment variable PG_PALETTE which points to the color yaml file
  2. You can use with_palette(f, file) or with_palette(f, load_palette(file)) and use it like with_logger. This doesn't work in a threaded context, see below.
  3. palette kwarg argument to functions such as plot_fuel or match_fuel_colors.

More tests may be necessary, since I discovered that the palette use was very confusing, and plotly and gr had its own options. It's a bit unclear what a valid color palette that supports both looks like.

In particular, the default order is hard coded in the get_default_palette used by plotly.

Dependency Update

Requires NREL-Sienna/PowerAnalytics.jl#9 to be merged first, as some filtering functions were fixed on the PowerAnalytics side.

I'd suggest merging the packages in the future, even if they are logically separate:

  • making changes always require changing both
  • PowerGraphics.jl tests can serve as a test for PowerAnalytics.jl

@josephmckinsey josephmckinsey marked this pull request as draft March 27, 2023 19:01
@josephmckinsey
Copy link
Contributor Author

A friend pointed out my code is not thread safe, so if we want concurrency, then global palette state will need to be significantly more clever. This applies if we use ENV or a package global variable.

It's apparently well-known that ENV is not thread-safe and gets propagated to contexts in weird ways:
JuliaLang/julia#34726 (comment)

There's apparently a library to fix this, along with a proposal to put it in stdlib, but it seems to have stalled for now. JuliaLang/julia#35833 and https://github.com/tkf/ContextVariablesX.jl

@josephmckinsey josephmckinsey marked this pull request as ready for review August 3, 2023 18:25
Project.toml Outdated
@@ -26,7 +26,7 @@ DataFrames = "1"
DataStructures = "0.18"
InfrastructureSystems = "1"
Plots = "1"
PowerAnalytics = "0.3"
PowerAnalytics = "^0.3.3"
Copy link
Member

Choose a reason for hiding this comment

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

Bump to 0.4 once it is registered check JuliaRegistries/General#89039

@@ -11,6 +11,7 @@ using PowerGraphics
using PowerAnalytics
using PlotlyJS
using PowerSimulations
using StorageSystemsSimulations
using GLPK
Copy link
Member

Choose a reason for hiding this comment

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

add HydroPowerSimulations.jl

@jd-lara jd-lara merged commit eb35e2c into main Aug 4, 2023
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.

2 participants