Skip to content

Conversation

@mashhoorahdal
Copy link

@mashhoorahdal mashhoorahdal commented Jan 23, 2026

Description

This PR fixes issue #207 by exporting version attributes from the stixpy.net.attrs module.

Changes Made

  • stixpy/net/attrs.py: Added all version attributes (Version, VersionU, MinVersion, MinVersionU, MaxVersion, MaxVersionU) to the module's __all__ list
  • changelog/209.bugfix.rst: Added changelog fragment documenting the bug fix
  • stixpy/net/tests/test_client.py: Added comprehensive tests to verify version attributes can be imported and function correctly

Testing

  • Added unit tests that verify all version attributes are accessible via import
  • Tests confirm version matching functionality works for all attribute types
  • Tests verify support for uncompleted versions (V02U format)

Issue Reference

Fixes #207: Version attrs not exported in all

Checklist

  • Tests pass
  • Changelog fragment added

@mashhoorahdal mashhoorahdal force-pushed the fix-version-attrs-export branch 3 times, most recently from 4ad07ea to 2777ba5 Compare January 23, 2026 06:51
@mashhoorahdal mashhoorahdal marked this pull request as draft January 26, 2026 17:21
@mashhoorahdal mashhoorahdal marked this pull request as ready for review January 26, 2026 17:21
@samaloney
Copy link
Member

So that issue was referring to the Fido attrs in the file indicated below, not the package version.

https://github.com/TCDSolar/stixpy/blob/main/stixpy/net/attrs.py

@codecov
Copy link

codecov bot commented Jan 27, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 81.53%. Comparing base (5a635ed) to head (bdba7c4).
⚠️ Report is 2 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #209   +/-   ##
=======================================
  Coverage   81.53%   81.53%           
=======================================
  Files          36       36           
  Lines        2459     2459           
=======================================
  Hits         2005     2005           
  Misses        454      454           

☔ View full report in Codecov by Sentry.
📢 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.

Added missing version attributes (Version, VersionU, MinVersion, MinVersionU, MaxVersion, MaxVersionU) to the __all__ list in stixpy.net.attrs module and added comprehensive tests to verify their functionality and importability.
@mashhoorahdal mashhoorahdal force-pushed the fix-version-attrs-export branch from 2964525 to 2f17004 Compare January 27, 2026 17:32
@mashhoorahdal
Copy link
Author

So that issue was referring to the Fido attrs in the file indicated below, not the package version.

https://github.com/TCDSolar/stixpy/blob/main/stixpy/net/attrs.py

Fixed the PR @samaloney

Copy link
Author

@mashhoorahdal mashhoorahdal left a comment

Choose a reason for hiding this comment

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

Added missing version attributes in __all__

@samaloney
Copy link
Member

Great thanks this looks good.

One of the main reasons is the the __all__ control what function etc are included in the generated documentation and now these are e.g. https://stixpy--209.org.readthedocs.build/en/209/reference/net.html#module-stixpy.net.attrs

Once the tests pass will get this merged

Changed the operator from less-than (<) to less-than-or-equal (<=) in both MaxVersion and MaxVersionU classes to correctly handle maximum version constraints. This ensures that the specified maximum version is included in the valid range rather than excluded.
Copy link
Author

@mashhoorahdal mashhoorahdal left a comment

Choose a reason for hiding this comment

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

Make MaxVersion attributes inclusive

Summary

MaxVersion and MaxVersionU were previously implemented as exclusive upper bounds using a strict < comparison.
This caused MaxVersion(n) to reject version Vnn, which is unintuitive and inconsistent with how version constraints are typically interpreted.

With the availability of newer STIX data versions (e.g. V03), this off-by-one behavior became visible and resulted in incorrect attribute matching.


What changed

  • Updated MaxVersion and MaxVersionU to use inclusive comparison (<=)
  • Ensures:
    • MaxVersion(3) matches V03
    • MaxVersion(3) does not match V04
  • Tests explicitly document and enforce this boundary behavior

Rationale

  • The name MaxVersion implies an inclusive upper bound
  • Rejecting Vnn for MaxVersion(n) is surprising to users
  • Inclusive behavior aligns with real STIX data usage
  • Tests act as executable documentation of intended semantics

Tests

Boundary cases are now covered:

  • MaxVersion(n) matches Vnn
  • MaxVersion(n) rejects versions greater than Vnn
  • MaxVersionU follows the same inclusive logic for uncompleted versions

Backward compatibility

This change is a bug fix, not a breaking change:

  • Existing valid queries continue to work
  • Previously failing boundary matches now behave correctly

@mashhoorahdal
Copy link
Author

@samaloney Why CI / core failing

Copy link
Contributor

@nicHoch nicHoch left a comment

Choose a reason for hiding this comment

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

looks all good

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.

Version attrs not exported in underscore all

3 participants