-
Notifications
You must be signed in to change notification settings - Fork 224
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 beachball offsetting with dict/pandas inputs #2202
Conversation
a63525c
to
2b167a9
Compare
Which line is it in https://github.com/GenericMappingTools/gmt/blob/6.4.0/src/seis/pscoupe.c? |
psmeca and pscoupe read "newX", "newY" and "title" from |
kwargs["A"] = True | ||
if "event_name" in spec.columns: | ||
newcols += ["event_name"] | ||
spec["event_name"] = spec["event_name"].astype(str) | ||
# reorder columns in DataFrame | ||
spec = spec.reindex(newcols, axis=1) | ||
elif isinstance(spec, np.ndarray) and spec.ndim == 1: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm, I was re-reading #1129 again and was gonna say, it's a little surprising that a 2D numpy array passed in with plot_longitude
and plot_latitude
columns (stored in float) works (since the test_meca_spec_2d_array
unit test is passing fine). I guess the GMT C code is smart enough with handling data from virtualfile_from_matrix
vs virtualfile_from_vectors
.
Nothing to do here, just a random comment.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it's a little surprising that a 2D numpy array passed in with
plot_longitude
andplot_latitude
columns (stored in float) works (since thetest_meca_spec_2d_array
unit test is passing fine).
That's a good point. I think it shouldn't work. The test_meca_spec_2d_array
test passes because both plot_longitude
and plot_latitude
are 0 (which means no offsetting due to backward compatibility issues).
I'll find some time and see if offsetting using a 2D numpy array works, and if not, will try to find a fix.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah yes, so we should probably update the test to use a non-zero offset. I actually wanted to parse all the user inputs into a pandas.DataFrame
(which can hold different dtypes like int, str, float) in #1613 (comment) which meant everything goes through virtualfile_from_vectors
, but that was superseded by #1784 that maintained two separate paths going via virtualfile_from_vectors
for dict
/pd.DataFrame
or virtualfile_from_matrix
for 2D np.ndarray
inputs. Just as an idea 🙂
If this ends up getting too complicated, we can merge in this PR first to fix offsets for dict/pandas inputs, and have a separate one to handle offsets for 2D numpy array inputs.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If this ends up getting too complicated, we can merge in this PR first to fix offsets for dict/pandas inputs, and have a separate one to handle offsets for 2D numpy array inputs.
I prefer to work on it in a separate PR.
Co-authored-by: Wei Ji <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok to merge, but don't close #2016 yet since offsets aren't working properly for 2d numpy array inputs.
…cMappingTools#2202) Co-authored-by: Wei Ji <[email protected]>
Description of proposed changes
Address #2016.
Due to how the GMT API works, the "plot_longitude", "plot_latitude", "event_name" columns must be in string type before passing to the GMT API.
The original code doesn't convert the columns to string type when these parameters are stored in dict/pandas inputs.
Reminders
make format
andmake check
to make sure the code follows the style guide.doc/api/index.rst
.Slash Commands
You can write slash commands (
/command
) in the first line of a comment to performspecific operations. Supported slash commands are:
/format
: automatically format and lint the code/test-gmt-dev
: run full tests on the latest GMT development version