-
Notifications
You must be signed in to change notification settings - Fork 229
**Breaking**: data_kind: data is None and required now returns the 'empty' kind #3482
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
Merged
Merged
Changes from 11 commits
Commits
Show all changes
30 commits
Select commit
Hold shift + click to select a range
0c82b3c
data_kind: Refactor the if-else statements into if-return statements
seisman 808755d
data_kind: Now 'matrix' represents a 2-D numpy array and unrecognizd …
seisman 0eb4f8f
Make 'data' a required parameter
seisman 9891b2c
Fix x2sys_cross as pd.DataFrame is 'vectors' kind now
seisman a9d094c
Fix legend as now 'vectors' doesn't mean data is None
seisman a0e1848
data_kind: data is None and required now returns the 'none' kind
seisman 3d8be4d
Add docstrings for stringio
seisman 7c104a9
Merge branch 'data_kind/return' into refactor/data_kind
seisman 5790923
Merge branch 'main' into refactor/data_kind
seisman 6954c5d
Fix docstrings
seisman ddda3b9
Merge branch 'refactor/data_kind' into data_kind/vectors-none
seisman 9300ca3
Merge branch 'main' into refactor/data_kind
seisman 2701a4a
Merge branch 'main' into refactor/data_kind
seisman c3cb459
Merge branch 'refactor/data_kind' into data_kind/vectors-none
seisman 7fcf57f
Merge branch 'main' into refactor/data_kind
seisman 91eb1b6
Merge branch 'refactor/data_kind' into data_kind/vectors-none
seisman a1e67d3
Rename 'none' kind to 'empty'
seisman 991f688
Merge branch 'main' into refactor/data_kind
seisman c83c9f8
Merge branch 'refactor/data_kind' into data_kind/vectors-none
seisman ea9ddaa
Update pygmt/helpers/utils.py
seisman 6f55375
clib: Switch the order of if-conditions to improve the Session.call_m…
seisman 3252988
Fix the conversion error for pandas.Series with missing values in pan…
seisman 003d8a1
Add type hints for GMT anchor codes (#3459)
seisman edf80c0
clib.Session: Add type hints and reformat docstrings (part 1) (#3504)
seisman fde7901
clib.conversion: Add type hints and improve docstrings for dataarray_…
seisman 78fdfb1
Update pygmt/helpers/utils.py
seisman ebb3257
Merge branch 'main' into data_kind/vectors-none
seisman e2b47b6
Merge remote-tracking branch 'origin/data_kind/vectors-none' into dat…
seisman a469acf
Fix docstrings
seisman 74bc161
Remove duplicated doctest
seisman File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
Can we change to another word like "empty" to reduce confusion with
None
?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.
I have another idea to completely eliminate the "none" kind. I'll open a POC PR later.
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 turns out the idea can't eliminate the "none" kind. So let's focus on this PR again.
I want to emphasize that
data_kind(data=None)
returns"none"
, sokind == "none"
meansdata is None
, which IMHO is very intuitive. That's why I choose"none"
as the new kind name, but I'm also OK with a newkind
name if others think"empty"
is a better name.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.
The intention of
kind == "none"
is to indicate that the data is stored in thex
,y
,z
variables rather than indata
(i.e.data is None
means data is in xyz variables). I thought about naming it askind="xyz"
but since we are planning to support other 1d arrays likeintensity
,symbol
, etc (#1076), maybe we should call itkind="xyzarrays"
? Or I'm ok with usingkind="empty"
too, just don't want to use"none"
because it is confusing withNone
.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.
Although not documented, I think in our codes, when we talk about "arrays", we usually mean numpy arrays, so a better name is "xyzvectors", but I feel it's confusing with the
"vectors"
kind. So, renamenone
toempty
in a1e67d3.