Rename model_inputs to data_request with deprecation handling

[Copilot]

Rename model_inputs to data_request ✅

Summary

Successfully renamed model_inputs to data_request throughout the Hyrax codebase with full backward compatibility and deprecation warnings.

Completed Tasks

• Explore codebase to find all model_inputs occurrences (115+ references found)
• Implement deprecation pattern using Python's warnings.warn()
• Update core code in data_provider.py:
  • Add backward compatibility for model_inputs config key with deprecation warning
  • Update internal references and comments to use data_request
  • Update error messages and logs to reference data_request
  • Simplify logic based on code review feedback
  • Remove redundant unreachable code paths
• Update default configuration file (hyrax_default_config.toml)
• Update source files:
  • data_set_registry.py (error message)
  • hyrax_csv_dataset.py (docstring)
  • pytorch_ignite.py (comments)
  • engine.py (comments)
• Write comprehensive tests for backward compatibility:
  • Test deprecation warning when using model_inputs
  • Test new data_request works without warning
  • Test empty config error handling
  • Test both keys present (data_request takes priority)
  • All 26 data_provider tests passing
  • Improved test isolation by removing conflicting keys
• Update documentation (7 files: notebooks and RST):
  • Fixed incorrect "plural" comment in model_input_1.ipynb
• Update benchmarks (2 files)
• Fix all affected tests
• Run test suite: 191+ tests passed
• Code review completed and all feedback addressed
• Security scan: No vulnerabilities found

Changes Made

Core Functionality:

• data_provider.py: Added backward compatibility logic with deprecation warnings
  • Removed redundant elif branches that were unreachable
  • Simplified logic to handle empty configs properly
• hyrax_default_config.toml: Added [data_request] section with clear deprecation notice

Source Code:

• Updated 4 source files with new terminology in comments/docstrings

Tests:

• Added 4 tests for deprecation behavior:
  • Deprecation warning when using model_inputs
  • No warning when using data_request
  • Error handling for empty configs
  • Both keys present (priority and no warning)
• Improved test isolation by explicitly removing conflicting keys
• Updated 1 test for new error message
• All 26 data_provider tests passing

Documentation & Benchmarks:

• Updated all 7 documentation files
• Fixed incorrect "plural" comment in model_input_1.ipynb
• Updated both benchmark files

Security Summary

✅ CodeQL scan completed - no security vulnerabilities detected

Backward Compatibility

• All existing code using model_inputs continues to work
• Users see deprecation warning: "The [model_inputs] configuration key is deprecated and will be removed in a future version. Please use [data_request] instead."
• No breaking changes

Original prompt

---

This section details on the original issue you should resolve

<issue_title>Rename model_inputs to data_request</issue_title>
<issue_description>Rename model_inputs to data_request. Use the @deprecated decorator to wrap the old function and dispatch to the new function.</issue_description>

<agent_instructions>Some of the references to the name will occur in configuration files and documentation. These should also be renamed.</agent_instructions>

Comments on the Issue (you are @copilot in this section)

• Fixes Rename model_inputs to data_request #603

---

💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.

[Copilot]

Pull request overview

This pull request successfully renames model_inputs to data_request throughout the Hyrax codebase with comprehensive backward compatibility support.

Changes:

• Implemented deprecation logic with clear warning messages for users still using model_inputs
• Updated configuration files, error messages, docstrings, and comments to use new terminology
• Added comprehensive tests for backward compatibility and deprecation behavior
• Updated all documentation (notebooks, RST files) and benchmarks to use data_request

Reviewed changes

Copilot reviewed 17 out of 17 changed files in this pull request and generated 5 comments.

Show a summary per file

File	Description
src/hyrax/data_sets/data_provider.py	Core deprecation logic with fallback support for model_inputs, updated error messages and docstrings
src/hyrax/hyrax_default_config.toml	Added [data_request] section and marked [model_inputs] as deprecated
src/hyrax/data_sets/data_set_registry.py	Updated error message to reference data_request
src/hyrax/data_sets/hyrax_csv_dataset.py	Updated docstring example to use data_request
src/hyrax/pytorch_ignite.py	Updated comment to reference data_request
src/hyrax/verbs/engine.py	Updated comment to reference data_request
tests/hyrax/test_data_provider.py	Added three new tests for deprecation behavior and updated existing test assertions
tests/hyrax/test_pytorch_ignite.py	Updated error message assertion to check for data_request
docs/pre_executed/*.ipynb	Updated variable names and explanatory text in documentation notebooks
docs/getting_started.rst	Updated code examples to use data_request_definition
benchmarks/*.py	Updated benchmark configurations to use data_request

[codecov]

Codecov Report

❌ Patch coverage is 95.23810% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 62.37%. Comparing base (99d226c) to head (ebe6e10).
⚠️ Report is 1 commits behind head on main.

Files with missing lines	Patch %	Lines
src/hyrax/data_sets/data_provider.py	95.00%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #606      +/-   ##
==========================================
+ Coverage   62.31%   62.37%   +0.06%     
==========================================
  Files          53       53              
  Lines        5235     5244       +9     
==========================================
+ Hits         3262     3271       +9     
  Misses       1973     1973              

☔ 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.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[github-actions]

Before [99d226c]	After [34d1008]	Ratio	Benchmark (Parameter)
failed	240±4μs	n/a	data_request_benchmarks.DatasetRequestBenchmarks.time_request_all_data
failed	1.66G	n/a	vector_db_benchmarks.VectorDBInsertBenchmarks.peakmem_load_vector_db(16384, 'chromadb')
failed	3.72G	n/a	vector_db_benchmarks.VectorDBInsertBenchmarks.peakmem_load_vector_db(16384, 'qdrant')
failed	1000M	n/a	vector_db_benchmarks.VectorDBInsertBenchmarks.peakmem_load_vector_db(2048, 'chromadb')
failed	1.25G	n/a	vector_db_benchmarks.VectorDBInsertBenchmarks.peakmem_load_vector_db(2048, 'qdrant')
failed	910M	n/a	vector_db_benchmarks.VectorDBInsertBenchmarks.peakmem_load_vector_db(256, 'chromadb')
failed	944M	n/a	vector_db_benchmarks.VectorDBInsertBenchmarks.peakmem_load_vector_db(256, 'qdrant')
failed	897M	n/a	vector_db_benchmarks.VectorDBInsertBenchmarks.peakmem_load_vector_db(64, 'chromadb')
failed	907M	n/a	vector_db_benchmarks.VectorDBInsertBenchmarks.peakmem_load_vector_db(64, 'qdrant')
failed	9.55±0.05s	n/a	vector_db_benchmarks.VectorDBInsertBenchmarks.time_load_vector_db(16384, 'chromadb')

Click here to view all benchmarks.

[gitosaurus]

LGTM

[Copilot]

Pull request overview

Copilot reviewed 17 out of 17 changed files in this pull request and generated 2 comments.

[gitosaurus]

@copilot apply changes based on the comments in this thread

[drewoldag]

This all looks correct to me. If you haven't already, I would recommend running the getting_started notebook locally just to ensure that it runs through at least the train and infer steps.

To be clear there would be no need to add the output of that run to this PR, it would just be a local smoke check.

[gitosaurus]

This all looks correct to me. If you haven't already, I would recommend running the getting_started notebook locally just to ensure that it runs through at least the train and infer steps.

To be clear there would be no need to add the output of that run to this PR, it would just be a local smoke check.

Smoke test clear!