Add Feature validate_quantity by jwbear · Pull Request #1521 · hdmf-dev/hdmf

jwbear · 2026-06-26T00:37:01Z

ISSUE #531

Motivation

What was the reasoning behind this change? Please explain the changes briefly.

How to test the behavior?

Show how to reproduce the new behavior (can be a bug fix or a new feature)

Test FLAGS not in dictionary, integers, strings, and integers entered as strings eg "2"

Checklist

Did you update CHANGELOG.md with your changes?
Does the PR clearly describe the problem and the solution?
Have you reviewed our Contributing Guide?
Does the PR use "Fix #XXX" notation to tell GitHub to close the relevant issue numbered XXX when the PR is merged?

ISSUE #531

codecov · 2026-06-26T00:38:43Z

Codecov Report

❌ Patch coverage is 90.90909% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 93.14%. Comparing base (b3685cb) to head (f9fac43).

Files with missing lines	Patch %	Lines
src/hdmf/spec/spec.py	90.90%	1 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##              dev    #1521      +/-   ##
==========================================
- Coverage   93.22%   93.14%   -0.08%     
==========================================
  Files          41       41              
  Lines       10168    10186      +18     
  Branches     2100     2104       +4     
==========================================
+ Hits         9479     9488       +9     
- Misses        413      421       +8     
- Partials      276      277       +1

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

FIX #531

for more information, see https://pre-commit.ci

jwbear · 2026-06-29T22:28:21Z

Added validate quantity and tests for validate_quantity, updated change log, and converted to private static method. Added name as parameter as outside scope for validate_quantity.

oruebel · 2026-06-29T23:29:58Z

     'doc': 'The default name of this base storage container, used only if name is None', 'default': None},
    {'name': 'attributes', 'type': list, 'doc': 'the attributes on this group', 'default': list()},
    {'name': 'linkable', 'type': bool, 'doc': 'whether or not this group can be linked', 'default': True},
    {'name': 'quantity', 'type': (str, int), 'doc': 'the required number of allowed instance', 'default': 1},


Since the behavior for quantity has changed, it may be useful to clarify this also here in the doc for the parameter

This still seems correct as we simply expanded the handling inputs and not the required values.

Since we're updating how "quantity" is handled, this is a good time to make this docstring more informative. Could you update it to say something like f"the required number of instances. Must be a positive integer, a string representation of a positive integer, or one of the flag keys {list(FLAGS.keys())} or values {list(FLAGS.values())}."

Copilot

Pull request overview

This PR addresses Issue #531 by adding validation/normalization for spec quantity values so that numeric strings can be handled consistently (instead of being treated as wildcard/many quantities).

Changes:

Added centralized quantity validation in BaseStorageSpec (including numeric-string handling).
Added unit tests for invalid quantity inputs on GroupSpec.
Added a changelog entry documenting the fix.

Reviewed changes

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

File	Description
`src/hdmf/spec/spec.py`	Introduces `__validate_quantity` and applies it during `BaseStorageSpec` initialization.
`tests/unit/spec_tests/test_group_spec.py`	Adds regression tests for invalid `quantity` inputs.
`CHANGELOG.md`	Adds a release note entry for Issue #531.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

jwbear · 2026-06-29T23:48:05Z

+        Returns:
+            A validated quantity string.
+
+        Raises:
+            ValueError: If quantity is not in FLAGS or is not >=1.
+            TypeError: If quantity is not an integer.
+        """


True removed type error

The new docstring still says TypeError

jwbear · 2026-06-29T23:49:15Z

+    def test_quantity_with_string_float(self):
+        msg = ("Invalid quantity '3.5': must be greater than or equal to 1 or in '[?, *, +]'")
+        with self.assertRaisesWith(ValueError, msg):
+            GroupSpec(doc='A test group', name='MyGroup', quantity='3.5')
+


Agreed, no valid testing code section for quantity

A positive test like what is described would be good to add.

You can add it to test_dataset_spec.py where the other positive tests live currently

oruebel · 2026-06-29T23:41:58Z

The code looks overall good to me. I added a few suggestions mostly related to documentation and asked Copilot to review. I'll let @rly do final review and approvals.

jwbear · 2026-06-29T23:50:59Z

@oruebel Thanks, I updated everything and will push again based on the suggestions. Except I ddint see positive tests. copilot: "These tests cover invalid quantity strings, but they don't assert the new intended behavior from #531: numeric strings like '2' should be accepted and treated as integers (so '1' does not imply 'many'). Adding a positive-case regression test would prevent the original bug from reappearing."

FIX #531

rly · 2026-06-30T00:57:48Z

 - Hardened the GitHub Actions CI: added least-privilege `permissions` blocks, pinned actions to commit SHAs (or immutable release tags), deduplicated the test setup into a composite action, passed untrusted inputs through environment variables, and added a zizmor security audit of the workflows. @rly [#1518](https://github.com/hdmf-dev/hdmf/pull/1518)

 ### Fixed
+- Fix #531 Validate quantity specified with an integor or numeric string @jwbear [#1521]


Please add the URL to the end of this line so that #1521 renders as a Markdown link in the changelog

Also please make this past tense, and correct spelling of "integer". "Quantity" should be clarified to be GroupSpec and DatasetSpec quantity. We typically don't mention the issue number since the PR links back to the issue. Either way is fine. So something like:

- Fixed issue #531 where invalid values were accepted for `GroupSpec.quantity` and `DatasetSpec.quantity`. Now only positive integers, string representations of positive integers, and '?', '*', and '+' are allowed. @jwbear [#1521](https://github.com/hdmf-dev/hdmf/pull/1521)

Please update text to be more descriptive of the change to users. Something like the above

…rly

…rly suggestions pending 1 FIX #531

jwbear added 2 commits June 25, 2026 20:31

Add Feature validate_quantity

6933514

ISSUE #531

Merge branch 'dev' into j0

d6553a4

jwbear and others added 2 commits June 29, 2026 18:06

Add Feature validate_quantity

8a961a9

FIX #531

[pre-commit.ci] auto fixes from pre-commit.com hooks

4d0aac7

for more information, see https://pre-commit.ci

jwbear marked this pull request as ready for review June 29, 2026 22:28

oruebel reviewed Jun 29, 2026

View reviewed changes

Comment thread src/hdmf/spec/spec.py Outdated

oruebel reviewed Jun 29, 2026

View reviewed changes

Comment thread CHANGELOG.md Outdated

oruebel reviewed Jun 29, 2026

View reviewed changes

oruebel requested review from Copilot and rly June 29, 2026 23:32

Copilot started reviewing on behalf of oruebel June 29, 2026 23:33 View session

Copilot AI reviewed Jun 29, 2026

View reviewed changes

jwbear added 2 commits June 29, 2026 19:52

Add Feature validate_quantity, updated based on @oreubel suggestions

fa77550

FIX #531

Merge branch 'dev' into j0

bfef44c

rly reviewed Jun 30, 2026

View reviewed changes

Comment thread src/hdmf/spec/spec.py

rly reviewed Jun 30, 2026

View reviewed changes

Comment thread src/hdmf/spec/spec.py Outdated

jwbear added 2 commits June 30, 2026 11:49

Merge branch 'dev' into j0

2b38632

Add Feature validate_quantity, updated based on @oreubel suggestions, @…

5ca2654

…rly suggestions pending 1 FIX #531

rly reviewed Jun 30, 2026

View reviewed changes

Comment thread CHANGELOG.md Outdated

Apply changelog suggestion from @rly

f9fac43

Uh oh!

Conversation

jwbear commented Jun 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Motivation

How to test the behavior?

Checklist

Uh oh!

codecov Bot commented Jun 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

jwbear commented Jun 29, 2026

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

oruebel commented Jun 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jwbear commented Jun 29, 2026

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

jwbear commented Jun 26, 2026 •

edited

Loading

codecov Bot commented Jun 26, 2026 •

edited

Loading

oruebel commented Jun 29, 2026 •

edited

Loading