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

Wind/wave rose plot #240

Merged
merged 14 commits into from
Aug 17, 2023
Merged

Wind/wave rose plot #240

merged 14 commits into from
Aug 17, 2023

Conversation

ecomodeller
Copy link
Member

@ecomodeller ecomodeller commented Jul 4, 2023

Code supplied by @daniel-caichac-DHI

@ecomodeller
Copy link
Member Author

image

@daniel-caichac-DHI
Copy link
Collaborator

Maybe add an example notebook on how it would work with Modelskill?

@jsmariegaard
Copy link
Member

Maybe add an example notebook on how it would work with Modelskill?

Great idea - would you have some short sample data which is a bit more interesting than @ecomodeller example above?

@daniel-caichac-DHI
Copy link
Collaborator

You suggesting @ecomodeller data is not interesting? omg.
I am uploading some data now
wrapper_tests.zip

@ecomodeller
Copy link
Member Author

Thx @daniel-caichac-DHI for the data.

Now on to fixing the legend...
image

I would prefer a single legend with two columns, greyscale and color, instead of two separate legends with identical interval, thoughts?

@daniel-caichac-DHI
Copy link
Collaborator

I just copied the mood style, but feel free to modify if you feel like

@ecomodeller
Copy link
Member Author

State after refactoring WIP
image

@ecomodeller
Copy link
Member Author

image

@ecomodeller
Copy link
Member Author

Shouldn't the histogram line up with the directional sectors 🤔
image

@daniel-caichac-DHI
Copy link
Collaborator

Shouldn't the histogram line up with the directional sectors 🤔 image

I think it depends on the dir_step=30, that it is the number of deg-bins, now it is 30°, si it will never align with the 45°.
If you change it to dir_step=22.5 I think it will

@jsmariegaard
Copy link
Member

Would it not be better to call the function directional_rose() or rose_diagram() instead of wind_rose() as data may have nothing to do with wind?

@daniel-caichac-DHI
Copy link
Collaborator

Could be called just rose

@ecomodeller
Copy link
Member Author

ecomodeller commented Aug 7, 2023

Here is a screenshot from an interactive example using Shiny
image

It seems like the size of the Calm circle is a bit too small to fit the text. 🤔

@daniel-caichac-DHI
Copy link
Collaborator

daniel-caichac-DHI commented Aug 8, 2023

If I remember correctly there is a *kwarg to resize the calm size

https://github.com/DHI/modelskill/blob/db6571b3e6769045d269638c871a553d00f90bd6/modelskill/rose.py#L17C14-L17C14

maybe change the default?

@ecomodeller
Copy link
Member Author

Would it not be better to call the function directional_rose() or rose_diagram() instead of wind_rose() as data may have nothing to do with wind?

I think the type of plot is commonly referred to as a "Wind rose" since it is most often used for wind, even though it can be applied to waves and currents as well.

I asked ChatGPT:

The term "Wind Rose" is one of the most commonly used synonyms for a rose diagram, especially when referring to the representation of wind direction and frequency data. It's widely used in meteorology, climate studies, and other fields where the distribution of wind directions is of interest.

However, the specific terminology used can vary depending on the field of application and the type of data being represented. For geological or structural analysis, terms like "Circular Histogram," "Polar Plot," or "Rose Diagram" might be more prevalent. Similarly, in fields like statistics or data visualization, terms like "Circular Bar Chart" or "Circular Histogram" might be more commonly used.

In any case, the underlying concept of representing directional data in a circular format remains consistent across these various terms and synonyms.

@jsmariegaard
Copy link
Member

ChatGPT: "'Wind Rose' is one of the most commonly used synonyms for a rose diagram" - I guess that is a case for "rose diagram" 😆

@ecomodeller ecomodeller marked this pull request as ready for review August 10, 2023 06:48
Copy link
Member

@jsmariegaard jsmariegaard left a comment

Choose a reason for hiding this comment

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

Fantastic work - looks really good.

I have a few suggestions:

  • I think it would be nice if the data could just be array like. It seems that you need to explicitly give it a numpy array (at least in the notebook). Why not accept anything that could be made numpy array?
  • It misses a "figsize" argument - we usually have that and I was missing it when I tested
  • watermark doesn't seem to work (or maybe I just misunderstand what it does)?
  • maybe rename the argument "resize_calm" to "calm_size" so it looks more similar than the other calm arguments.



def test_wind_rose_image_identical(wave_data_model_obs, tmp_path):
# TODO this test seems fragile, since it relies pixel by pixel comparison of images
Copy link
Collaborator

Choose a reason for hiding this comment

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

I agree.
I have so far not found a way to compare the resulting plots.

Copy link
Member

@jsmariegaard jsmariegaard left a comment

Choose a reason for hiding this comment

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

:shipit:

@jsmariegaard jsmariegaard merged commit 6204929 into main Aug 17, 2023
9 checks passed
@jsmariegaard jsmariegaard deleted the rose branch August 17, 2023 19:24
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