Refactor meca to use virtualfile_from_data #1613
Conversation
Need to ensure that the spec numpy.array is a 2D matrix for now, so that test_meca_spec_dictionary and test_meca_spec_1d_array works.
Remove huge chunk of if-then code block by converting Python dict to pandas.DataFrame, and handle the spec formatting using one route only.
| # or assemble the array for the case of pd.DataFrames | ||
| elif isinstance(dict_type_pointer, np.ndarray): | ||
| # Convert single int, float data to List[int, float] data | ||
| _spec = { | ||
| "longitude": np.atleast_1d(longitude), | ||
| "latitude": np.atleast_1d(latitude), | ||
| "depth": np.atleast_1d(depth), | ||
| } | ||
| _spec.update({key: np.atleast_1d(val) for key, val in spec.items()}) | ||
| spec = pd.DataFrame.from_dict(_spec) | ||
|
|
||
| assert isinstance(spec, pd.DataFrame) | ||
| dict_type_pointer = spec.values | ||
|
|
||
| # Assemble the array for the case of pd.DataFrames | ||
| if isinstance(dict_type_pointer, np.ndarray): |
There was a problem hiding this comment.
Just to explain this change (clearer if you look at commit e223057), previously the spec could be formatted in 3 different paths depending on the type:
if isinstance(dict_type_pointer, (int, float)):(i.e. a dictionary of int/float like{"depth": 0}elif isinstance(dict_type_pointer, list):(i.e. a dictionary of lists like{"depth": [0, 1, 2]}elif isinstance(dict_type_pointer, np.ndarray):(i.e. a pandas.DataFrame withnp.ndarraycolumns)
There was a lot of duplication in these 3 code paths, so I've removed paths 1 and 2 by converting any spec provided by the user in dictionary format into pandas.DataFrame. So now, everything happens via path 3 only.
Another benefit of handling everything using pandas.DataFrame is that columns can have different data types, so this sets things up for the ability to pass in string labels later in a follow up PR.
weiji14
changed the title
| @@ -247,6 +240,29 @@ def update_pointers(data_pointers): | |||
| ): | |||
There was a problem hiding this comment.
Lines 237-241 are unnecessary because line 236 already checks spec is dict or pd.DataFrame. Am I right?
There was a problem hiding this comment.
You might be right, let me recheck the logic.
|
Closing this in favour of #1784. |
Description of proposed changes
Part 1/3 of
mecarefactor as mentioned at #1129 (comment). Focus of this Pull Request is to clean up thepygmt.mecacode and add some unit tests so that more refactorings can take place later to fix bugs. Themecamodule could use a lot of refactoring in general, so feel free to suggest changes if it simplifies/improves the code!Notes:
offsetbug in pygmt.Figure.meca offset not working #1129 nor allow beachballs to be plotted with string labels, c.f. Wrap meca #516 (comment). Those will be done in a separate PR to keep this one small.Part of #949. Towards resolving the bug at #1129 and enhancement request at #1466.
Reminders
make formatandmake checkto 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