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

plot and plot3d: Allow passing an array as symbol #1076

Closed
core-man opened this issue Mar 18, 2021 · 15 comments · Fixed by #3559
Closed

plot and plot3d: Allow passing an array as symbol #1076

core-man opened this issue Mar 18, 2021 · 15 comments · Fixed by #3559
Assignees
Labels
feature request New feature wanted
Milestone

Comments

@core-man
Copy link
Member

core-man commented Mar 18, 2021

Description of the desired feature

We can now use a 1d array for sizes and color parameters, and intensity can also accept a 1d array after #1065. Therefore, we may also support a 1d array as symbol or symbols (originally posted in #1065 (comment)).


The format of the input file of GMT plot (including both -S and -I):

x y [ z ] [ size ] [ symbol-parameters ] [ intensity ] [ symbol ]

  • z corresponds to the color parameter in PyGMT
  • size corresponds to the sizes parameter in PyGMT (I don't know why we add a "s")
  • intensity corresponds to the intensity (the name may be updated) parameter in PyGMT (Allow passing an array as intensity for plot #1065 is working on it)

The format of the input file of GMT plot3d (including both -S and -I):

x y z [ w ] [ size ] [ symbol-parameters ] [ intensity ] [ symbol ]

Therefore, we'd also like to support an array as symbol when we use x/y in plot and x/y/z in plot3d.


See an example with x y z size intensity symbol for the GMT plot method.

gmt begin symbols png
gmt makecpt -Chot -T0/3/1
gmt plot -R0/5/0/1 -JX5c/1c -BWSne -Bxa1 -Bya0.5 -S -C -I -W1p,black << EOF
1  0.5   0   0.3  1  c
2  0.5   1   0.8 -1  t
3  0.5   2   0.5  0  i
EOF
gmt end show

symbols

Are you willing to help implement and maintain this feature? Yes, but volunteers are welcome.

@core-man core-man added the feature request New feature wanted label Mar 18, 2021
@weiji14
Copy link
Member

weiji14 commented Mar 23, 2021

A bit of a side issue, but do you think we can make a generic _plot function that shares some common code between plot and plot3d, as is being done with blockmean and blockmode at #1091?

@core-man
Copy link
Member Author

A bit of a side issue, but do you think we can make a generic _plot function that shares some common code between plot and plot3d, as is being done with blockmean and blockmode at #1091?

I quickly checked the current source codes of those two methods in PyGMT, which are the same except that plot3d has an additional z parameter. So I think it's a good suggestion to make a generic _plot function.

@seisman
Copy link
Member

seisman commented Mar 23, 2021

There are still many unimplemented and complicated features/options in plot and plot3d. I would prefer NOT to make the generic _plot function until we have more thoughts on the possible features and changes.

@core-man
Copy link
Member Author

There are still many unimplemented and complicated features/options in plot and plot3d. I would prefer NOT to make the generic _plot function until we have more thoughts on the possible features and changes.

That's true. Actually, I think I found two bugs when I try to work on this issue for the plot method. Will double-check before submitting them.

@core-man
Copy link
Member Author

For this issue, shall we create a new parameter called symbol or symbols (which one is better?) to accept 1d-array symbols or just use the style parameter instead? I prefer to have a new parameter, just like the sizes parameter (I dont' understand why we add a s at the end) of the plot/plot3d method. How do you think?

@seisman
Copy link
Member

seisman commented Mar 23, 2021

like the sizes parameter (I dont' understand why we add a s at the end) of the plot/plot3d method

plot is one of the oldest wrappers in PyGMT, and the parameters are not carefully chosen. We had some discussions in the GMT Community meeting, and I think we agree that we will change sizes to size.

shall we create a new parameter called symbol or symbols (which one is better?) to accept 1d-array symbols or just use the style parameter instead?

I'm not sure if we should reuse style or not. If you'd like to work on this feature, I think you can use symbol first, then we can decide which one is better based on your changes.

@core-man
Copy link
Member Author

plot is one of the oldest wrappers in PyGMT, and the parameters are not carefully chosen. We had some discussions in the GMT Community meeting, and I think we agree that we will change sizes to size.

Sound great.

shall we create a new parameter called symbol or symbols (which one is better?) to accept 1d-array symbols or just use the style parameter instead?

I'm not sure if we should reuse style or not. If you'd like to work on this feature, I think you can use symbol first, then we can decide which one is better based on your changes.

Great. I will try~

@seisman
Copy link
Member

seisman commented Mar 25, 2021

@core-man Thanks for your quick PR in #1117. Here are the possible ways to set symbols and sizes after your PR. I feel some of them are not intuitive to me.

  1. constant symbol and size: style="c0.5c"
  1. constant symbol and different sizes: style="cc" + sizes=sizes

Although we use style="cc" in our documentation, I still don't think it's not easy to understand.

  1. different symbols and constant size: style="0.5c" + symbol=symbol

I believe most users would use symbol=symbol + sizes="0.5c".

  1. different symbols and sizes: style=True + sizes=sizes + symbol=symbol

The style=True should NOT be required.

@core-man
Copy link
Member Author

core-man commented Mar 25, 2021

  1. constant symbol and different sizes: style="cc" + sizes=sizes

Although we use style="cc" in our documentation, I still don't think it's not easy to understand.

We can use style="c" for this case because the default unit is cetimeter. The unit can also be set by the PROJ_LENGTH_UNIT using the config method.

  1. different symbols and constant size: style="0.5c" + symbol=symbol

I believe most users would use symbol=symbol + sizes="0.5c".

That's true, but it seems that sizes cannot work for a single string or a 1d-array string now. We may refractor it. See the last comment below.

  1. different symbols and sizes: style=True + sizes=sizes + symbol=symbol

The style=True should NOT be required.

Good suggestion. Will include in #1117.


@seisman based on your comments, I think you are requesting that we should use symbol and sizes instead of style, which I also prefer because it is more intuitive. I will try to work on it.

@core-man
Copy link
Member Author

core-man commented Apr 13, 2021

  1. different symbols and constant size: style="0.5c" + symbol=symbol

I believe most users would use symbol=symbol + sizes="0.5c".

Shall we support a string 1d-array for the sizes parameter? Now it only accepts int and float (e.g., 1, 0.5) and does not accept string (e.g., 0.5c), although we can set the unit using the config method.

  1. different symbols and sizes: style=True + sizes=sizes + symbol=symbol

The style=True should NOT be required.

Fixed in 1c48b39. See #1117 (comment). We only need to use symbol=symbol, sizes=sizes.

@weiji14
Copy link
Member

weiji14 commented May 18, 2021

Just following up on this. I think having 3 parameters (style/symbol/size) will get too confusing. If 'style' is not a good alias name for -S, perhaps we should deprecate it using @deprecate_parameter and rename 'style' to 'symbol' as hinted by @core-man at #1117 (comment)? I.e. keep only 2 parameters (symbol and size).

@core-man
Copy link
Member Author

Just following up on this. I think having 3 parameters (style/symbol/size) will get too confusing. If 'style' is not a good alias name for -S, perhaps we should deprecate it using @deprecate_parameter and rename 'style' to 'symbol' as hinted by @core-man at #1117 (comment)? I.e. keep only 2 parameters (symbol and size).

Good idea: First, deprecate style to symbol first, and then we could work on the remaining two parameters, i.e., symbol and size. Any comment @seisman?

@weiji14
Copy link
Member

weiji14 commented Jun 8, 2023

@seisman, considering that meca supports longitude, latitude, depth, plot_longitude, plot_latitude, event_name as lists/arrays since the refactor in #1784 by putting everything into a spec dict and turning it into a pandas.DataFrame, could we do something similar here for plot/plot3d to handle symbols? This chunk of code in particular:

pygmt/pygmt/src/meca.py

Lines 319 to 332 in 6c69fb6

# override the values in dict/pd.DataFrame if parameters are explicity
# specified
if longitude is not None:
spec["longitude"] = np.atleast_1d(longitude)
if latitude is not None:
spec["latitude"] = np.atleast_1d(latitude)
if depth is not None:
spec["depth"] = np.atleast_1d(depth)
if plot_longitude is not None:
spec["plot_longitude"] = np.atleast_1d(plot_longitude)
if plot_latitude is not None:
spec["plot_latitude"] = np.atleast_1d(plot_latitude)
if event_name is not None:
spec["event_name"] = np.atleast_1d(event_name).astype(str)

The spec dict is then converted to a pandas.DataFrame, and passed in to lib.virtualfile_from_data via the data parameter here:

file_context = lib.virtualfile_from_data(check_kind="vector", data=spec)

Maybe we could create a reusable function out of this somehow?

@seisman
Copy link
Member

seisman commented Jun 8, 2023

Maybe we could create a reusable function out of this somehow?

Sounds a promising idea.

@seisman
Copy link
Member

seisman commented Oct 29, 2024

Implemented in PR #1117 and #3559.

@seisman seisman closed this as completed Oct 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature request New feature wanted
Projects
None yet
3 participants