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

Figure.meca: Fix the bug when passing a dict of scalar values to the spec parameter #2174

Merged
merged 8 commits into from
Nov 13, 2022

Conversation

seisman
Copy link
Member

@seisman seisman commented Oct 28, 2022

Description of proposed changes

Minimal example:

import pygmt

fig = pygmt.Figure()
fig.meca(
    spec=dict(strike=0, dip=90, rake=0, magnitude=5, longitude=0, latitude=5, depth=0),
    scale="2.5c",
    region=[-1, 1, 4, 6],
    projection="M14c",
    frame=2,
)
fig.savefig("meca.png")

Running the example above produces the following error messages:

Traceback (most recent call last):
  File "/Users/seisman/OSS/gmt/pygmt/workspace/test2.py", line 4, in <module>
    fig.meca(
  File "/Users/seisman/OSS/gmt/pygmt/pygmt/helpers/decorators.py", line 578, in new_module
    return module_func(*args, **kwargs)
  File "/Users/seisman/OSS/gmt/pygmt/pygmt/helpers/decorators.py", line 718, in new_module
    return module_func(*args, **kwargs)
  File "/Users/seisman/OSS/gmt/pygmt/pygmt/src/meca.py", line 286, in meca
    spec = pd.DataFrame(spec)
  File "/Users/seisman/opt/miniconda/lib/python3.9/site-packages/pandas/core/frame.py", line 636, in __init__
    mgr = dict_to_mgr(data, index, columns, dtype=dtype, copy=copy, typ=manager)
  File "/Users/seisman/opt/miniconda/lib/python3.9/site-packages/pandas/core/internals/construction.py", line 502, in dict_to_mgr
    return arrays_to_mgr(arrays, columns, index, dtype=dtype, typ=typ, consolidate=copy)
  File "/Users/seisman/opt/miniconda/lib/python3.9/site-packages/pandas/core/internals/construction.py", line 120, in arrays_to_mgr
    index = _extract_index(arrays)
  File "/Users/seisman/opt/miniconda/lib/python3.9/site-packages/pandas/core/internals/construction.py", line 664, in _extract_index
    raise ValueError("If using all scalar values, you must pass an index")
ValueError: If using all scalar values, you must pass an index

The error is raised when we try to convert a dict with scalar values to pandas.DataFrame. So, here is the real minimal example to reproduce the issue:

import pandas as pd
pd.DataFrame(dict(strike=0, dip=90, rake=0, magnitude=5, longitude=0, latitude=5, depth=0))

The solution here is to convert dict's values to ndarray before the converison.

Reminders

  • Run make format and make check to make sure the code follows the style guide.
  • Add tests for new features or tests that would have caught the bug that you're fixing.
  • Add new public functions/methods/classes to doc/api/index.rst.
  • Write detailed docstrings for all functions/methods.
  • If wrapping a new module, open a 'Wrap new GMT module' issue and submit reasonably-sized PRs.
  • If adding new functionality, add an example to docstrings or tutorials.

Slash Commands

You can write slash commands (/command) in the first line of a comment to perform
specific operations. Supported slash commands are:

  • /format: automatically format and lint the code
  • /test-gmt-dev: run full tests on the latest GMT development version

@seisman seisman added the bug Something isn't working label Oct 28, 2022
@seisman seisman added this to the 0.8.0 milestone Oct 28, 2022
@seisman seisman force-pushed the meca-all-scalar-values branch from 170600d to efde646 Compare October 28, 2022 14:50
@seisman seisman added the needs review This PR has higher priority and needs review. label Oct 31, 2022
@seisman
Copy link
Member Author

seisman commented Nov 10, 2022

Ping @GenericMappingTools/pygmt-maintainers for review.

Copy link
Member

@michaelgrund michaelgrund left a comment

Choose a reason for hiding this comment

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

Looks good!

@seisman seisman added final review call This PR requires final review and approval from a second reviewer and removed needs review This PR has higher priority and needs review. labels Nov 11, 2022
pygmt/src/meca.py Outdated Show resolved Hide resolved
pygmt/src/meca.py Outdated Show resolved Hide resolved
# Right lateral strike slip focal mechanism
fig.meca(
spec=dict(
strike=0, dip=90, rake=0, magnitude=5, longitude=0, latitude=5, depth=0
Copy link
Member

Choose a reason for hiding this comment

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

Have you tested passing in the event_name name as a string too? Want to see if np.at_least1d handles it properly.

Copy link
Member Author

Choose a reason for hiding this comment

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

It works, because np.atleast_1d("abc") returns array(['abc'], dtype='<U3').

I've revised the test to also pass a scalar event_name argument.

pygmt/tests/test_meca.py Show resolved Hide resolved
@seisman seisman merged commit ecdb66c into main Nov 13, 2022
@seisman seisman deleted the meca-all-scalar-values branch November 13, 2022 01:15
@seisman seisman changed the title meca: Fix the bug when passing a dict of scalar values to the spec parameter Figure.meca: Fix the bug when passing a dict of scalar values to the spec parameter Nov 13, 2022
@seisman seisman removed the final review call This PR requires final review and approval from a second reviewer label Nov 13, 2022
sixy6e pushed a commit to sixy6e/pygmt that referenced this pull request Dec 21, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants