Conversation
Review fixes on the stereonet module: - Fix a segfault: a bare -W (no pen argument) together with -Tl left W.string NULL while W.active was already true, so aliasing the pen onto -L called strdup(NULL). - Reject dips/plunges outside 0-90 and rakes outside 0-180. Such values projected onto the far hemisphere, where they are clipped away, so the module quietly produced a figure with missing data instead of an error. - Accept a negative rake as the usual shorthand for a rake measured from the opposite end of the strike, folding it into the 0-180 range (-25 is read as 155), matching the convention used by mplstereonet. The docs previously claimed negatives were rejected, which was never enforced. - Restore test/geology/minimal.sh as a classic-mode test producing $ps so the test harness can compare it, and drop the minimal.png/minimal.txt artifacts that were committed by accident.
New modules must add themselves to four separate hardcoded name lists in gmt_modern.c, or their modern name silently misbehaves in ways that only show up once you specifically go looking (this is not mentioned in devdocs/custom_supplements.rst): - gmt_current_name(): without this, every GMT_Report message (errors, warnings, info) prints the classic name "psstereonet" even when the user typed "stereonet" and is in modern mode. - gmtlib_get_active_name(): same classic->modern translation, used by a different set of callers. - gmtlib_is_modern_name(): without this, "gmt stereonet -^" (or any bare invocation outside a session, e.g. querying usage) fails outright with "Shared GMT module not found: stereonet", because the modern-mode leniency that lets a bare modern name grab usage/purpose text before a session exists never triggers. Found by testing "gmt stereonet ..." against the exact same scenarios for core module ternary and supplement modules polar/coupe (same pattern, already correctly registered) and diffing the behavior.
Esteban82
added a commit
that referenced
this pull request
Sep 25, 2026
Adds a WARNING heading under the module title in both doc pages, matching the style already used in gmt_update.rst, since the module (PR #9178) is still WIP and its syntax/defaults may still change. Tested with: docutils parse of both files before/after — identical set of pre-existing include/substitution notices (expected outside a full Sphinx build), no new warnings or errors from the inserted section. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Adds a WARNING heading under the module title in both doc pages, matching the style already used in gmt_update.rst, since the module (PR #9178) is still WIP and its syntax/defaults may still change. Tested with: docutils parse of both files before/after — identical set of pre-existing include/substitution notices (expected outside a full Sphinx build), no new warnings or errors from the inserted section. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…o stereonet Per Joaquim's review comment on issue #9156: a single-module addition gets its own top-level src/ directory named after itself (windbarbs is not meteorology/windbarbs), not wrapped in an unnecessary broader subject name. Renames src/geology, test/geology and doc/rst/source/supplements/geology to stereonet, and THIS_MODULE_LIB/SUPPL_NAME to match. No functional change; the module still lands in the same supplements plugin either way. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Esteban82
force-pushed
the
stereonet-module
branch
from
September 26, 2026 02:08
55d1fdd to
b0ecebe
Compare
- A failed internal psxy/psbasemap call returned API->error, which is 0 by then, so the module reported the error but exited 0; return the code psxy actually gave instead. - Symbols that read their size or parameters from extra data columns (vectors, -Sc without a size, -Se without axes, ...) silently drew nothing, since plot only ever gets lon, lat [and z]; reject them up front via gmt_parse_symbol_option instead. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
+r measured a positive rake downward from the strike, the opposite sign of Aki & Richards (what GMT's meca calls rake), and folded negative rakes by adding 180, which matches neither convention: an A&R rake of -30 on plane 000/45 is the line 022/21, but +r plotted it at 158/21. It was never documented, so remove it here and bring it back in its own PR with a documented, verified convention. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
-C (color by a third column) and -M (dump the lon/lat of traces or poles) were implemented but missing from usage() and both RST pages. Also say that -S must carry its size and parameters, add a Notes item on giving -JA? in each subplot panel, define bedding.txt in the second example, and fix the -bi column count. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The geology->stereonet rename made each row two characters wider than its grid-table border, so docutils flags both tables as malformed, and left the entries in geology's old alphabetical slot. Fix the widths and move the toctree, list and table entries after spotter. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Every other entry there compares strlen+2 characters, i.e., the whole name; 9U only matched the prefix, so e.g. "stereonetxyz" counted too. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Comment-only change (identical code after stripping comments and whitespace): cut the multi-line rationale blocks to one line each and the module header down to a brief synopsis plus the credits. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This branch has not been deployed
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Opening this as a draft/WIP specifically to get feedback before going further. @rhum1-geo, your suggestions (density contouring, Fisher stats, notation auto-detection, richer -S/vector options) are noted and intentionally not in this first PR — the plan is to land plotting first, then follow up with statistics in a later PR once the plotting side is settled.
What's implemented:
-T[d|l|p]— planes as strike/dip (right-hand rule) or dip-direction/dip, or lines as trend/plunge-W/-S/-L/-G— cyclographic traces and poles (or lines), styled like plot-A/-B— azimuth ring and the net's own grid mesh, both opt-in-T+u— upper hemisphereTested with (no automated tests yet — these are the scripts I manually ran and checked; anyone from #9156 is welcome to run them too):
0_Empty_Nets.sh— empty Schmidt and Wulff nets, with title, grid, and azimuth ring, no data1_Plot_Data.sh— a plane + its pole from strike/dip (RHR) input, styled independently2_Plot_Data_Inverted.sh— same data with columns swapped;-:and-i1,0should reproduce script 1 exactly3_Plot_Planes_Conventions.sh— the same plane entered as-Tp(strike/dip, RHR) and-Td(dip-direction/dip); traces should coincide4_Plot_Poles_Conventions.sh—-Tpand-Tdpoles for the same two planes should coincide (red under blue);-Tlreuses the same numbers as a line's trend/plunge instead, which is a different quantity and lands elsewhere on purpose5_Annotations.sh— fractional azimuth interval (-A22.5) with a title and-ViModel/effort: Claude Opus 5 for design and code review, Claude Sonnet 5 for iteration and testing.
Related to #9156.