Conversation
chore: sync dev with main
…fy MD031 Fixes #10. Root cause: pymarkdown's MD031 fixer has a bug with fenced code blocks nested inside list items. It tries to add blank lines around the fence but also strips the 3-space indent, which un-nests the block from its list item. On the container runner it crashes outright (BadPluginError); on newer local versions it "succeeds" but produces corrupted markdown (e.g. replacing "git tag -f -a latest" with no indent, pulling it out of the numbered step). Workaround: preemptively apply the correct form — blank line before the fence, fence still at 3-space indent — so the fixer has nothing to do. Also clean up MD022 (missing blanks around headings), MD026 (trailing colons), MD032 (missing blanks around lists), and MD034 (bare URLs in TEMPERATURE_UNITS_IMPLEMENTATION.md). With both files now idempotent under pymarkdown fix + scan, the pre-commit exclusion list drops back to the original 3 files. The underlying pymarkdown fixer bug should be reported upstream. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Introduces docs/decisions/ with a MADR-ish template (status, context, decision, consequences, alternatives, upgrade trigger) and the first ADR documenting why derived chemistry quantities like molar mass live as computed @Property accessors on Material rather than in a new ChemicalProperties group. Rationale (short version): molar mass is definitionally a function of formula; storing it invites drift. Rust crate rs-materials already exposes it as a computed function — Python parity matters for Monte Carlo consumers that use both. YAGNI rules out a 1-field property group. Upgrade trigger: when ≥2 non-definitionally-derived chemistry properties (heat of formation, oxidation state, etc.) need a home, introduce the group and keep Material.molar_mass as a shortcut property reading from it. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…er docs Fixes #9. enrich_from_periodictable no longer promises to set density for compounds — periodictable only has density data for pure elements, and a compound's density depends on crystal packing (not derivable from formula). The docstring now explicitly says so and points users at enrich_from_matproj for compound density. The density-setting code path is unchanged (it was already a no-op for compounds because formula.density is None); the fix is removing the false promise and removing the three latently-broken tests that expected it to work. In its place, Material gains a computed molar_mass @Property (and molar_mass_qty for pint). Molar mass is parsed from self.formula using a local ATOMIC_WEIGHT table in pymat/elements.py — mirror of mat-rs/src/elements.rs to keep Python and Rust APIs symmetric for Monte Carlo consumers using both. The new parser handles fractional stoichiometry (Lu1.8Y0.2SiO5) and strips dopant notation (LYSO:Ce). See ADR-0001 for why this lives on Material rather than in a ChemicalProperties group. Tests: - Three previously xfailed tests in test_enrichers.py rewritten to check composition extraction + molar_mass (what actually works), no longer xfail. - New TestMolarMass class in test_core.py with 8 tests: pure element, simple compound, fractional stoichiometry, dopant stripping, missing formula, unknown element, pint integration. - 125 passed, 11 skipped (up from 122 passed, 3 xfailed). Also: enricher uses logger.warning instead of print for invalid formula warnings, and properly chains ImportError via `from e`. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
fix: enricher molar_mass + pymarkdown MD031 + ADR infrastructure
Fixes #11. The root cause of the 3.12 pin was that CI ran `uv sync --all-extras` on a single Python, so the narrowest optional-extra's Python support (`vtk` and `cadquery-ocp`, both cp311/cp312-only, pulled in via `build123d`) became the project's effective Python support. `py-materials`'s only hard dependency is `pint`; the build123d integration is optional, and the core library works on 3.13+. Changes: - **pyproject.toml**: environment markers on `build123d`, `dev`, and `all` extras — `build123d>=0.7.0; python_version<'3.13'`. Installers silently drop the dep on 3.13+ instead of erroring on a missing wheel. Also add `Python :: 3.13` classifier. - **uv.lock**: regenerated to reflect the markers. - **ci.yml**: new `test-matrix` job with two cells: - `py3.12, all` — full integration including build123d - `py3.13, core` — `--extra periodictable --extra matproj`, skips build123d-dependent tests A `test` roll-up job with `needs: [test-matrix]` keeps the `Tests` context name stable, so branch rulesets don't need updating. Matrix cells appear under `Tests (py3.12, all)` / `Tests (py3.13, core)`. - **setup-env/action.yml**: new `python-version` input (overrides `.python-version` when set) and `uv-sync-args` input (lets matrix cells pass `--extra ...` instead of the fixed `--all-extras`). - **.python-version**: left at `3.12` for local dev convenience (covers all-extras out of the box). Users who want 3.13 for core work can override via `UV_PYTHON`. Verified locally: 133 pass / 11 skip on 3.12 all-extras; 107 pass / 26 skip on 3.13 core (the 26 skips are build123d-gated tests, which correctly skip when the dep is absent). A build123d regression stays loud because the 3.12 cell still runs the full integration suite — the original concern about "break build123d and not notice immediately" is preserved. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
On 3.13, --all-extras correctly skips build123d (via env marker) and still installs pytest, periodictable, matproj. No need to enumerate --extra ... explicitly. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
ci: matrix Python 3.12+all-extras and 3.13+core (closes #11)
The previous `Detect merge conflicts` step ran `git merge --no-commit --no-ff origin/main 2>/dev/null` and branched on exit code to set `conflict=true|false`. Two problems: 1. `2>/dev/null` hid git's stderr entirely, so whenever the step misfired we got no diagnostic — just a `(conflicts)` label on a PR that was actually clean. 2. `git merge --no-ff` has degenerate behavior when one side is an ancestor of the other. After a dev→main merge PR, `main`'s tip is a merge commit whose second parent is `dev`'s old tip, so `dev` is an ancestor of `main`. `git merge --no-ff` in this configuration can exit non-zero without an actual conflict, which the workflow then mislabels. This was observed live on PR #13 (the sync-back from the initial dev→main bootstrap): the PR had 0 files changed and was fully mergeable, but arrived labeled `merge-conflict` and titled "(conflicts)". Replace with `git merge-tree --write-tree origin/dev origin/main`, which does an in-memory trial merge without touching the working tree or index. Exit semantics: - 0 = clean merge, output is the merged tree SHA - 1 = conflicts, output contains conflict markers - anything else = real error (propagate) Stderr is now captured into a variable rather than dropped, so any future misbehavior is visible in the run log. This pattern is lifted from the current `vig-os/devcontainer/assets/workspace/.github/workflows/sync-main-to-dev.yml` upstream — confirming this isn't a novel fix but an already-corrected upstream implementation that our stale scaffolded copy never picked up. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
fix(sync): use git merge-tree for false-positive-free conflict detection
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
1 participant
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.
Summary
Second dev → main sync under the new rulesets. Catches `main` up with three merged PRs on `dev`.
Rolls up:
Test plan
🤖 Generated with Claude Code