Refactors ARTIS reader into dedicated module structure #3356

wkerzendorf · 2025-12-10T15:22:32Z

Reorganizes ARTIS-related code by moving it from a single reader file into a dedicated module with separate data and reader components.

Extracts the ArtisModelData class into a standalone data module and renames it to ArtisData for consistency.

Improves code organization and maintainability by separating data structures from reading functionality.

Reorganizes ARTIS-related code by moving it from a single reader file into a dedicated module with separate data and reader components. Extracts the ArtisModelData class into a standalone data module and renames it to ArtisData for consistency. Improves code organization and maintainability by separating data structures from reading functionality.

Moves ARTIS-related functionality into a dedicated module structure and updates import paths accordingly. Reorganizes imports in the ARTIS data module for better readability and fixes docstring reference to use correct class name. Adds missing newline at end of file to follow Python conventions.

Copilot

Pull request overview

This PR refactors the ARTIS reader code by moving it from a single file into a dedicated module structure. The main changes separate data structures from reading functionality and rename the data class for consistency.

Key Changes:

Extracted ArtisModelData class into a new data.py module and renamed it to ArtisData
Updated import statements across the codebase to reference the new module structure
Separated data structure definitions from reader implementation

Reviewed changes

Copilot reviewed 5 out of 8 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
tardis/io/tests/test_model_reader.py	Updated import to reference new module path for `read_artis_density`
tardis/io/model/readers/base.py	Updated import to reference new module path for `read_artis_density`
tardis/io/model/artis/tests/test_artis_readers.py	Updated import to reference new module path for ARTIS readers
tardis/io/model/artis/readers.py	Removed `ArtisModelData` class definition and updated to use `ArtisData` from new data module
tardis/io/model/artis/data.py	New file containing `ArtisData` class extracted from readers module

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

Copilot · 2025-12-10T15:23:11Z

tardis/io/model/artis/data.py

+    def to_geometry(self):
+        """
+        Construct a HomologousRadial1DGeometry object from this ArtisData.
+        points for the shells. The time_of_model is used as the time_explosion.


The docstring is incomplete. Line 20 appears to be a fragment that should be part of a complete sentence explaining that velocity values are treated as boundary points for the shells.

Suggested change

points for the shells. The time_of_model is used as the time_explosion.

The velocity values are treated as boundary points for the shells, and

the time_of_model is used as the time_explosion.

tardis-bot · 2025-12-10T15:25:02Z

*beep* *bop*
Hi human,
I ran ruff on the latest commit (881a449).
Here are the outputs produced.
Results can also be downloaded as artifacts here.
Summarised output:

Details

20769	      	[ ] syntax-error
 14	W291  	[ ] trailing-whitespace
 10	S108  	[ ] hardcoded-temp-file
  9	W293  	[ ] blank-line-with-whitespace
  8	I001  	[*] unsorted-imports
  7	F821  	[ ] undefined-name
  5	E902  	[ ] io-error
  5	F405  	[ ] undefined-local-with-import-star-usage
  4	W292  	[ ] missing-newline-at-end-of-file
  4	B018  	[ ] useless-expression
  4	ICN001	[ ] unconventional-import-alias
  4	UP015 	[*] redundant-open-modes
  3	E402  	[ ] module-import-not-at-top-of-file
  3	INP001	[ ] implicit-namespace-package
  2	F401  	[*] unused-import
  1	E701  	[ ] multiple-statements-on-one-line-colon
  1	E703  	[*] useless-semicolon
  1	E712  	[ ] true-false-comparison
  1	COM818	[ ] trailing-comma-on-bare-tuple
  1	F631  	[ ] assert-tuple
  1	PGH003	[ ] blanket-type-ignore
  1	PGH004	[ ] blanket-noqa
  1	PT009 	[ ] pytest-unittest-assertion
  1	UP009 	[*] utf8-encoding-declaration
  1	UP030 	[ ] format-literals
  1	UP032 	[*] f-string
Found 20862 errors.
[*] 21 fixable with the `--fix` option (4 hidden fixes can be enabled with the `--unsafe-fixes` option).

Complete output(might be large):

Details

docs/conf.py:1:1: UP009 [*] UTF-8 encoding declaration is unnecessary
docs/conf.py:27:1: I001 [*] Import block is un-sorted or un-formatted
docs/conf.py:30:8: F401 [*] `tardis` imported but unused
docs/conf.py:35:1: I001 [*] Import block is un-sorted or un-formatted
docs/conf.py:40:43: PGH004 Use specific rule codes when using `noqa`
docs/conf.py:50:27: UP015 [*] Unnecessary mode argument
docs/conf.py:71:1: F405 `exclude_patterns` may be undefined, or defined from star imports
docs/conf.py:72:1: F405 `exclude_patterns` may be undefined, or defined from star imports
docs/conf.py:73:1: F405 `exclude_patterns` may be undefined, or defined from star imports
docs/conf.py:74:1: F405 `exclude_patterns` may be undefined, or defined from star imports
docs/conf.py:75:1: F405 `exclude_patterns` may be undefined, or defined from star imports
docs/conf.py:143:1: W293 Blank line contains whitespace
docs/conf.py:178:1: W293 Blank line contains whitespace
docs/conf.py:195:13: UP030 Use implicit references for positional format fields
docs/conf.py:195:13: UP032 [*] Use f-string instead of `format` call
docs/conf.py:222:1: E402 Module level import not at top of file
docs/conf.py:294:4: E712 Avoid equality comparisons to `True`; use `toml_config_tool_dict["tardis"]['edit_on_github']:` for truth checks
docs/conf.py:316:1: E402 Module level import not at top of file
docs/conf.py:368:1: E402 Module level import not at top of file
docs/conf.py:383:37: UP015 [*] Unnecessary modes, use `w`
docs/conf.py:398:37: UP015 [*] Unnecessary modes, use `w`
tardis/io/model/artis/readers.py:162:89: PGH003 Use specific rule codes when ignoring type issues
tardis/io/model/artis/tests/test_artis_readers.py:1:1: INP001 File `tardis/io/model/artis/tests/test_artis_readers.py` is part of an implicit namespace package. Add an `__init__.py`.
tardis/io/model/artis/tests/test_artis_readers.py:50:5: PT009 Use a regular `assert` instead of unittest-style `assert_`
tardis/io/model/artis/tests/test_artis_readers.py:68:5: F631 Assert test is a non-empty tuple, which is always `True`
tardis/io/model/readers/tests/test_ascii_readers.py:1:1: INP001 File `tardis/io/model/readers/tests/test_ascii_readers.py` is part of an implicit namespace package. Add an `__init__.py`.
tardis/io/model/readers/tests/test_model_reader.py:1:1: INP001 File `tardis/io/model/readers/tests/test_model_reader.py` is part of an implicit namespace package. Add an `__init__.py`.
Found 27 errors.
[*] 8 fixable with the `--fix` option (4 hidden fixes can be enabled with the `--unsafe-fixes` option).

tardis-bot · 2025-12-10T16:03:17Z

*beep* *bop*
Hi human,
I ran benchmarks as you asked comparing master (73b41fc) and the latest commit (881a449).
Here are the logs produced by ASV.
Results can also be downloaded as artifacts here.

Significantly changed benchmarks:

Details

All benchmarks:

Details

Benchmarks that have stayed the same:

| Change   | Before [73b41fc1] <master>   | After [881a4496]    | Ratio   | Benchmark (Parameter)                                                                                                               |
|----------|------------------------------|---------------------|---------|-------------------------------------------------------------------------------------------------------------------------------------|
|          | 44.4±20μs                    | 57.9±20μs           | ~1.30   | transport_montecarlo_interaction.BenchmarkTransportMontecarloInteraction.time_line_scatter                                          |
|          | 1.40±0.6μs                   | 1.64±0.4μs          | ~1.17   | transport_geometry_calculate_distances.BenchmarkTransportGeometryCalculateDistances.time_calculate_distance_line                    |
|          | 52.1±0.4ms                   | 57.5±1ms            | ~1.10   | transport_montecarlo_packet_trackers.BenchmarkTransportMontecarloPacketTrackers.time_rpacket_trackers_to_dataframe                  |
|          | 25.0±6μs                     | 21.9±5μs            | ~0.88   | transport_montecarlo_packet_trackers.BenchmarkTransportMontecarloPacketTrackers.time_generate_rpacket_last_interaction_tracker_list |
|          | 561±100ns                    | 581±200ns           | 1.04    | opacities_opacity.BenchmarkMontecarloMontecarloNumbaOpacities.time_photoabsorption_opacity_calculation                              |
|          | 3.58±0.01ms                  | 3.71±0.01ms         | 1.04    | opacities_opacity_state.BenchmarkOpacitiesOpacityState.time_opacity_state_initialize('macroatom')                                   |
|          | 2.52±0.8ms                   | 2.60±0.7ms          | 1.03    | transport_montecarlo_single_packet_loop.BenchmarkTransportMontecarloSinglePacketLoop.time_single_packet_loop                        |
|          | 1.06±0μs                     | 1.07±0μs            | 1.01    | spectrum_formal_integral.BenchmarkTransportMontecarloFormalIntegral.time_intensity_black_body                                       |
|          | 38.2±0.7μs                   | 38.5±0.1μs          | 1.01    | transport_montecarlo_packet_trackers.BenchmarkTransportMontecarloPacketTrackers.time_generate_rpacket_tracker_list                  |
|          | 481±200ns                    | 480±200ns           | 1.00    | opacities_opacity.BenchmarkMontecarloMontecarloNumbaOpacities.time_compton_opacity_calculation                                      |
|          | 59.2±0.01s                   | 59.0±0s             | 1.00    | spectrum_formal_integral.BenchmarkTransportMontecarloFormalIntegral.time_FormalIntegrator_functions                                 |
|          | 699±2ns                      | 699±0.4ns           | 1.00    | transport_montecarlo_interaction.BenchmarkTransportMontecarloInteraction.time_thomson_scatter                                       |
|          | 1.57±0μs                     | 1.56±0μs            | 1.00    | transport_montecarlo_vpacket.BenchmarkMontecarloMontecarloNumbaVpacket.time_trace_bad_vpacket                                       |
|          | 7.29±0.2μs                   | 7.28±0.6μs          | 1.00    | transport_montecarlo_vpacket.BenchmarkMontecarloMontecarloNumbaVpacket.time_trace_vpacket_volley                                    |
|          | 1.65±0μs                     | 1.64±0μs            | 1.00    | transport_montecarlo_vpacket.BenchmarkMontecarloMontecarloNumbaVpacket.time_trace_vpacket_within_shell                              |
|          | 1.09±0m                      | 1.08±0m             | 0.99    | run_tardis.BenchmarkRunTardis.time_run_tardis_rpacket_tracking                                                                      |
|          | 20.1±0.1μs                   | 19.9±0.03μs         | 0.99    | transport_montecarlo_interaction.BenchmarkTransportMontecarloInteraction.time_line_emission                                         |
|          | 3.44±0μs                     | 3.40±0μs            | 0.99    | transport_montecarlo_vpacket.BenchmarkMontecarloMontecarloNumbaVpacket.time_trace_vpacket                                           |
|          | 39.6±0.01s                   | 38.5±0.01s          | 0.97    | run_tardis.BenchmarkRunTardis.time_run_tardis                                                                                       |
|          | 2.31±1μs                     | 2.25±0.9μs          | 0.97    | transport_montecarlo_estimators_radfield_estimator_calcs.BenchmarkMontecarloMontecarloNumbaPacket.time_update_line_estimators       |
|          | 521±200ns                    | 501±200ns           | 0.96    | opacities_opacity.BenchmarkMontecarloMontecarloNumbaOpacities.time_pair_creation_opacity_calculation                                |
|          | 1.27±0μs                     | 1.22±0μs            | 0.96    | transport_geometry_calculate_distances.BenchmarkTransportGeometryCalculateDistances.time_calculate_distance_boundary                |
|          | 2.70±0.02ms                  | 2.56±0.01ms         | 0.95    | opacities_opacity_state.BenchmarkOpacitiesOpacityState.time_opacity_state_initialize('scatter')                                     |
|          | 1.77±0.01ms                  | 1.66±0.01ms         | 0.94    | transport_montecarlo_main_loop.BenchmarkTransportMontecarloMontecarloMainLoop.time_montecarlo_main_loop                             |

If you want to see the graph of the results, you can check it here

… signatures, and restructuring test fixtures for clarity and maintainability.

Consolidates scattered how-to documentation from multiple locations into a unified structure under docs/how_to/. Moves guides from docs/io/ subdirectories and docs/how-to/ into organized categories including read_external_models, output, grid, custom_source, and code_comparison. Removes automatic generation of how-to guides page in favor of manual organization with proper table of contents structure. Updates import paths in notebooks to reflect new module organization and fixes circular import issue in artis readers.

review-notebook-app · 2025-12-10T18:11:26Z

Check out this pull request on

See visual diffs & provide feedback on Jupyter Notebooks.

Powered by ReviewNB

tardis-bot · 2025-12-10T18:20:56Z

*beep* *bop*

Hi, human.

The docs workflow has succeeded ✔️

Click here to see your results.

andrewfullard · 2025-12-10T18:50:11Z

docs/how_to/code_comparison/plasma_compare/tardis_example.yml

This file rename may be causing the docs build to fail.

you mean from how-to to how_to?

It was complaining about the missing tardis_example in the build failure

docs/how_to/read_external_models/how_to_read_artis_model.ipynb

andrewfullard · 2025-12-10T18:53:52Z

tardis/io/model/artis/readers.py

        Combined data with time_of_model, velocity, mean_density, and mass_fractions.
    """
-    time_of_model, velocity, mean_density, isotope_mass_fractions = read_artis_density(
+    from tardis.io.model.artis.data import ArtisData


Why is the import here?

it was trying to avoid a circular import. Let me see where this is coming from.

…ations

Removes example configuration and cleans notebook outputs Deletes unused TARDIS configuration file from plasma comparison documentation and clears executed cell outputs in ARTIS model reading notebook for cleaner repository state

…source notebook

Moves ARTIS-specific test configuration files to the appropriate module directory structure to improve organization and maintainability. Creates a dedicated fixture for accessing ARTIS test data directory, replacing hardcoded paths in test classes. Adds missing import formatting for better code style consistency.

Moves import statement to avoid circular dependency between base reader and ARTIS module. Updates type annotation to use direct class reference instead of string literal for better type checking.

codecov · 2025-12-10T22:07:23Z

Codecov Report

❌ Patch coverage is 92.59259% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 68.21%. Comparing base (73b41fc) to head (881a449).
⚠️ Report is 1 commits behind head on master.

Files with missing lines	Patch %	Lines
tardis/io/model/artis/data.py	85.71%	2 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #3356      +/-   ##
==========================================
- Coverage   68.45%   68.21%   -0.24%     
==========================================
  Files         178      179       +1     
  Lines       13617    13624       +7     
==========================================
- Hits         9321     9294      -27     
- Misses       4296     4330      +34

☔ 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.

wkerzendorf added 2 commits December 10, 2025 10:17

Copilot AI review requested due to automatic review settings December 10, 2025 15:22

Copilot AI reviewed Dec 10, 2025

View reviewed changes

wkerzendorf added 2 commits December 10, 2025 11:26

Enhances ARTIS data handling by adding type hints, improving function…

055e2c0

… signatures, and restructuring test fixtures for clarity and maintainability.

andrewfullard reviewed Dec 10, 2025

View reviewed changes

wkerzendorf added 5 commits December 10, 2025 14:24

Adds new Jupyter notebook for converting SNEC data to TARDIS configur…

572cb0a

…ations

Removes unnecessary markdown cell and cleans up notebook metadata

bc64772

Adds wget command to download TARDIS example configuration in custom …

f2067f9

…source notebook

andrewfullard added the restructure label Dec 10, 2025

wkerzendorf added 2 commits December 10, 2025 15:51

Fixes circular import in model readers

18c110f

Moves import statement to avoid circular dependency between base reader and ARTIS module. Updates type annotation to use direct class reference instead of string literal for better type checking.

Merge remote-tracking branch 'upstream/master' into restructure/io/artis

881a449

wkerzendorf mentioned this pull request Dec 10, 2025

Very simple atomic restructure and clean-up #3357

Merged

andrewfullard approved these changes Dec 11, 2025

View reviewed changes

andrewfullard requested a review from atharva-2001 December 11, 2025 02:50

wkerzendorf merged commit 9cbb564 into tardis-sn:master Dec 11, 2025
25 of 26 checks passed

	points for the shells. The time_of_model is used as the time_explosion.
	The velocity values are treated as boundary points for the shells, and
	the time_of_model is used as the time_explosion.

Uh oh!

Refactors ARTIS reader into dedicated module structure #3356

Refactors ARTIS reader into dedicated module structure #3356

Conversation

wkerzendorf commented Dec 10, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Dec 10, 2025

Choose a reason for hiding this comment

Uh oh!

tardis-bot commented Dec 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

tardis-bot commented Dec 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

review-notebook-app bot commented Dec 10, 2025

Uh oh!

tardis-bot commented Dec 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

andrewfullard Dec 10, 2025

Choose a reason for hiding this comment

Uh oh!

wkerzendorf Dec 10, 2025

Choose a reason for hiding this comment

Uh oh!

andrewfullard Dec 10, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

andrewfullard Dec 10, 2025

Choose a reason for hiding this comment

Uh oh!

wkerzendorf Dec 10, 2025

Choose a reason for hiding this comment

Uh oh!

codecov bot commented Dec 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

tardis-bot commented Dec 10, 2025 •

edited

Loading

tardis-bot commented Dec 10, 2025 •

edited

Loading

tardis-bot commented Dec 10, 2025 •

edited

Loading

codecov bot commented Dec 10, 2025 •

edited

Loading