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

Update output shape and mean values from some x2sys_cross tests #2986

Merged
merged 3 commits into from
Jan 16, 2024

Conversation

weiji14
Copy link
Member

@weiji14 weiji14 commented Jan 11, 2024

Description of proposed changes

Small changes to the number of output rows, and the mean value of the z_X and z_M/z_1/z_2 columns.

Also for test_x2sys_cross_input_dataframe_with_nan, a deep copy of the tracks: list[pd.DataFrame] object is now made before NaN values are set, to avoid some test flakiness.

Addresses #2961 (comment)

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.
  • Use underscores (not hyphens) in names of Python files and directories.

Slash Commands

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

  • /format: automatically format and lint the code

So that the modified tracks won't be used in other test functions, causing tests to fail.
Small changes to the number of output rows, and the mean value of the z_X and z_M/z_1/z_2 columns.
@weiji14 weiji14 added the maintenance Boring but important stuff for the core devs label Jan 11, 2024
@weiji14 weiji14 added this to the 0.11.0 milestone Jan 11, 2024
@weiji14 weiji14 self-assigned this Jan 11, 2024
@weiji14 weiji14 mentioned this pull request Jan 11, 2024
14 tasks
Comment on lines +146 to +148
newtracks = copy.deepcopy(x=tracks)
newtracks[0].loc[newtracks[0]["z"] < -15, "z"] = np.nan # set some NaN values
output = x2sys_cross(tracks=newtracks, tag=tag, coe="i")
Copy link
Member Author

Choose a reason for hiding this comment

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

@seisman, this is similar to your commit f97d92e in #2936, but I used a deep copy (copy.deepcopy()) instead of a shallow copy (.copy()) because setting NaNs on the shallow copy newtracks still modified the original tracks object.

Comment on lines 66 to +69
output = x2sys_cross(tracks=["@tut_ship.xyz"], tag=tag, coe="i")

assert isinstance(output, pd.DataFrame)
assert output.shape == (14294, 12)
assert output.shape == (14338, 12)
Copy link
Member Author

Choose a reason for hiding this comment

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

Still a little unsure why GMT 6.5.0's x2sys_cross returns more rows than GMT 6.4.0. Might need to investigate.

@weiji14 weiji14 marked this pull request as ready for review January 16, 2024 01:44
@weiji14 weiji14 added the needs review This PR has higher priority and needs review. label Jan 16, 2024
@weiji14
Copy link
Member Author

weiji14 commented Jan 16, 2024

Haven't got time to check whether the change in output shape from x2sys_cross in GMT 6.5.0 mentioned at #2986 (comment) is ok or not, but gonna mark this ready for review so that we can merge this and get our test suite back to green.

@seisman seisman merged commit d15c79f into main Jan 16, 2024
12 of 18 checks passed
@seisman seisman deleted the fix/x2sys_cross_gmt6.5 branch January 16, 2024 02:50
@seisman seisman removed the needs review This PR has higher priority and needs review. label Jan 16, 2024
@seisman
Copy link
Member

seisman commented Jan 27, 2024

Haven't got time to check whether the change in output shape from x2sys_cross in GMT 6.5.0 mentioned at #2986 (comment) is ok or not, but gonna mark this ready for review so that we can merge this and get our test suite back to green.

The x2sys_cross output change is caused by upstream PR GenericMappingTools/gmt#8188. So the new output is correct.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
maintenance Boring but important stuff for the core devs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants