Skip to content

Conversation

@seisman
Copy link
Member

@seisman seisman commented Jul 31, 2025

The parameter separator is a little too long, use sep instead.

sep is commonly used in the Python world, e.g., in str.split and pandas.read_csv.

sequence_jion was added in v0.16.0 so it's a breaking change, but I guess very few users will use sequence_join. The separator parameter is marked as deprecated and will be removed in v0.19.0.

@seisman seisman added this to the 0.17.0 milestone Jul 31, 2025
@seisman seisman added maintenance Boring but important stuff for the core devs deprecation Deprecating a feature labels Jul 31, 2025
@seisman seisman requested a review from Copilot July 31, 2025 05:12
@seisman seisman added the needs review This PR has higher priority and needs review. label Jul 31, 2025
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR deprecates the separator parameter in favor of the shorter sep parameter in sequence_join function and related alias functionality. The change aligns with common Python conventions used in libraries like pandas and the standard library.

  • Adds deprecation warnings for separator parameter usage
  • Updates function signatures to use sep as the primary parameter
  • Updates all existing code and documentation to use the new sep parameter

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File Description
pygmt/tests/test_alias_system.py Updates test code to use sep instead of separator
pygmt/helpers/utils.py Implements deprecation logic and updates sequence_join function signature
pygmt/alias.py Updates _to_string and Alias class to use sep parameter

@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 Jul 31, 2025
@seisman seisman merged commit 3788d90 into main Jul 31, 2025
25 of 26 checks passed
@seisman seisman deleted the separator-sep branch July 31, 2025 08:37
@seisman seisman removed the final review call This PR requires final review and approval from a second reviewer label Jul 31, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

deprecation Deprecating a feature maintenance Boring but important stuff for the core devs

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants