Skip to content

Align release documentation with current behavior - #315

Merged
mishugeb merged 5 commits into
masterfrom
dev
Sep 28, 2026
Merged

mishugeb merged 5 commits into
masterfrom
dev

Conversation

@mishugeb

Copy link
Copy Markdown
Collaborator

Readme updated

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot review overview

🟡 Changes recommended

README and input documentation still contain inconsistent seed and batch semantics.

Review effort: Lite
Findings: 1 Low severity

Open (1)
What changed in this PR

Aligns release documentation with current defaults, seed handling, GPU batching, and output behavior.

Changes:

  • Updates maximum_signatures default to 25.
  • Documents root-seed reproducibility and Seeds.txt.
  • Clarifies GPU batching and records related fixes.
File Description
README.md Updates API defaults and parameter documentation.
docs/​3. Using the Tool - Input.md Updates seed-file guidance.
docs/​4. Using the Tool - Output.md Documents the new Seeds.txt format.
CHANGELOG.md Records current behavioral changes.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread README.md Outdated

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot review overview

🔵 Needs a closer look

The seed-file example image contradicts the newly documented single-root-seed format.

Review effort: Balanced
Findings: None

Resolved since last review (1)
Previously missed (1)

In code that hasn't changed since last review

Low severity Update seed-file example image to show single-root-seed format

docs/​4. Using the Tool - Output.md:29

The example image immediately below still shows the previous multi-row replicate-ID/seed layout, which contradicts the newly documented single-root-seed format. Regenerate that image or replace it with a one-row example so users do not construct a file that read_seed_file rejects.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot review overview

🟡 Changes recommended

The Python API docstring and CLI help still contradict the updated seed and GPU-batching documentation.

Review effort: Balanced
Findings: 1 Low severity

Open (1)

Comment thread README.md
| | **nmf_replicates** | Positive Integer | The number of iteration to be performed to extract each number signature (default: `100`). |
| | **resample** | Boolean | If `True`, add poisson noise to samples by resampling (default: `True`). |
| | **seeds** | String | Ensures reproducible NMF replicate resamples. Provide the path to the `Seeds.txt` file (found in the results folder from a previous analysis) to reproduce results (default: `"random"`). |
| | **seeds** | String | Ensures reproducible NMF resampling and initialization. Provide the path to a `Seeds.txt` file containing exactly one root seed in the `Seed` column (the file generated by a previous analysis can be reused) to reproduce results (default: `"random"`). |

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot review overview

🟢 Approval recommended

The documentation matches current behavior overall, with one non-blocking changelog wording issue noted.

Review effort: Balanced
Findings: 1 Low severity

Open (1)
Previously missed (1)

In code that hasn't changed since last review

Low severity Missing Seed column error lacks a specific diagnostic

CHANGELOG.md:33

read_seed_file() converts a missing-column KeyError into only ValueError("Please set valid seeds") (sigpro.py:304-308), so the diagnostic does not identify the missing Seed column. This release note overstates the clarity of that validation; either add a column-specific error in the implementation or describe it simply as validation.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot review overview

🟢 Approval recommended

The documentation matches the verified implementation, and the validation change has focused test coverage.

Review effort: Balanced
Findings: 1 Low severity

Open (1)

@mishugeb
mishugeb merged commit 44d7db2 into master Sep 28, 2026
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants