config: remove configwritermixin class #2921
Conversation
|
*beep* *bop* Hi, human. I'm the @tardis-bot and couldn't find your records in my database. I think we don't know each other, or you changed your credentials recently. Please add your name and email to In case you need to map an existing alias, follow this example. |
|
*beep* *bop* 2 G004 [ ] Logging statement uses f-string
2 RET505 [ ] Unnecessary `else` after `return` statement
1 INP001 [ ] File `tardis/io/configuration/tests/test_config_reader.py` is part of an implicit namespace package. Add an `__init__.py`.
1 E999 [ ] SyntaxError: Expected an expression
1 W292 [*] No newline at end of file
1 F401 [*] `pandas` imported but unused
Complete output(might be large): .mailmap:1:38: E999 SyntaxError: Expected an expression
.mailmap:292:39: W292 [*] No newline at end of file
tardis/io/configuration/config_reader.py:52:29: G004 Logging statement uses f-string
tardis/io/configuration/config_reader.py:116:9: RET505 Unnecessary `else` after `return` statement
tardis/io/configuration/config_reader.py:140:13: RET505 Unnecessary `else` after `return` statement
tardis/io/configuration/config_reader.py:206:29: G004 Logging statement uses f-string
tardis/io/configuration/tests/test_config_reader.py:1:1: INP001 File `tardis/io/configuration/tests/test_config_reader.py` is part of an implicit namespace package. Add an `__init__.py`.
tardis/io/configuration/tests/test_config_reader.py:2:18: F401 [*] `pandas` imported but unused
Found 8 errors.
[*] 2 fixable with the `--fix` option.
|
|
*beep* *bop* Significantly changed benchmarks: All benchmarks: Benchmarks that have stayed the same:
| Change | Before [7e37d5fe] <master> | After [a45999a0] | Ratio | Benchmark (Parameter) |
|----------|------------------------------|---------------------|---------|-------------------------------------------------------------------------------------------------------------------------------------|
| | 572±100ns | 792±200ns | ~1.38 | opacities_opacity.BenchmarkMontecarloMontecarloNumbaOpacities.time_photoabsorption_opacity_calculation |
| | 3.20±0.8μs | 4.00±0.8μs | ~1.25 | transport_montecarlo_vpacket.BenchmarkMontecarloMontecarloNumbaVpacket.time_trace_vpacket_within_shell |
| | 2.95±0.6μs | 3.56±0.5μs | ~1.21 | transport_montecarlo_vpacket.BenchmarkMontecarloMontecarloNumbaVpacket.time_trace_bad_vpacket |
| | 6.92±1μs | 7.85±2μs | ~1.13 | transport_montecarlo_vpacket.BenchmarkMontecarloMontecarloNumbaVpacket.time_trace_vpacket |
| | 2.50±2μs | 2.18±2μs | ~0.87 | transport_montecarlo_estimators_radfield_estimator_calcs.BenchmarkMontecarloMontecarloNumbaPacket.time_update_line_estimators |
| | 69.5±20μs | 43.0±40μs | ~0.62 | transport_montecarlo_interaction.BenchmarkTransportMontecarloInteraction.time_line_emission |
| | 77.7±30μs | 47.0±40μs | ~0.60 | transport_montecarlo_interaction.BenchmarkTransportMontecarloInteraction.time_line_scatter |
| | 33.7±10μs | 36.9±10μs | 1.09 | transport_montecarlo_packet_trackers.BenchmarkTransportMontecarloPacketTrackers.time_generate_rpacket_last_interaction_tracker_list |
| | 4.47±0.01ms | 4.83±0.1ms | 1.08 | opacities_opacity_state.BenchmarkOpacitiesOpacityState.time_opacity_state_initialize('macroatom') |
| | 1.82±0.05ms | 1.95±0.03ms | 1.07 | transport_montecarlo_main_loop.BenchmarkTransportMontecarloMontecarloMainLoop.time_montecarlo_main_loop |
| | 1.15±0μs | 1.22±0μs | 1.06 | transport_geometry_calculate_distances.BenchmarkTransportGeometryCalculateDistances.time_calculate_distance_boundary |
| | 712±0.5ns | 753±0.9ns | 1.06 | transport_montecarlo_interaction.BenchmarkTransportMontecarloInteraction.time_thomson_scatter |
| | 7.41±2μs | 7.89±1μs | 1.06 | transport_montecarlo_vpacket.BenchmarkMontecarloMontecarloNumbaVpacket.time_trace_vpacket_volley |
| | 2.95±0.4ms | 3.09±0.4ms | 1.05 | transport_montecarlo_single_packet_loop.BenchmarkTransportMontecarloSinglePacketLoop.time_single_packet_loop |
| | 39.5±0.04μs | 41.1±0.8μs | 1.04 | transport_montecarlo_packet_trackers.BenchmarkTransportMontecarloPacketTrackers.time_generate_rpacket_tracker_list |
| | 591±100ns | 601±200ns | 1.02 | opacities_opacity.BenchmarkMontecarloMontecarloNumbaOpacities.time_pair_creation_opacity_calculation |
| | 1.07±0m | 1.09±0m | 1.02 | run_tardis.BenchmarkRunTardis.time_run_tardis_rpacket_tracking |
| | 2.10±0m | 2.14±0m | 1.02 | spectrum_formal_integral.BenchmarkTransportMontecarloFormalIntegral.time_FormalIntegrator_functions |
| | 203±0.1ns | 206±0.1ns | 1.01 | spectrum_formal_integral.BenchmarkTransportMontecarloFormalIntegral.time_intensity_black_body |
| | 1.62±0.4μs | 1.64±0.2μs | 1.01 | transport_geometry_calculate_distances.BenchmarkTransportGeometryCalculateDistances.time_calculate_distance_line |
| | 531±100ns | 531±100ns | 1.00 | opacities_opacity.BenchmarkMontecarloMontecarloNumbaOpacities.time_compton_opacity_calculation |
| | 66.6±0.4ms | 66.7±0ms | 1.00 | transport_montecarlo_packet_trackers.BenchmarkTransportMontecarloPacketTrackers.time_rpacket_trackers_to_dataframe |
| | 3.60±0.05ms | 3.48±0.02ms | 0.97 | opacities_opacity_state.BenchmarkOpacitiesOpacityState.time_opacity_state_initialize('scatter') |
| | 41.8±0.04s | 40.6±0.08s | 0.97 | run_tardis.BenchmarkRunTardis.time_run_tardis |
If you want to see the graph of the results, you can check it here |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #2921 +/- ##
==========================================
- Coverage 70.40% 70.00% -0.41%
==========================================
Files 221 221
Lines 16147 16135 -12
==========================================
- Hits 11369 11295 -74
- Misses 4778 4840 +62 ☔ View full report in Codecov by Sentry. |
| expected.to_hdf(hdf_file_path, overwrite=True) | ||
| actual = pd.read_hdf(hdf_file_path, key="/simulation/config") | ||
| expected = expected.get_properties()["config"] | ||
| assert actual[0] == expected[0] |
There was a problem hiding this comment.
Why did you remove this test?
There was a problem hiding this comment.
Hi,
I've tried to pass HDFWriterMixin as parent class to Configuration inorder to utilise get_properties. However when this particular test runs theres an attribute error about hdf_properties and other variables because they are not defined in Configuration class. this was not caught in the original code because we were overwriting get_properties.
There was a problem hiding this comment.
Hi, can I get a clarification for this. So that I can make necessary changes.
Type: : 🎢
infrastructureCloses #2321
How did you test these changes?
☑️ Checklist
build_docslabel@andrewfullard