Adds a missing import in custom_abundance.py #2942

Sonu0305 · 2025-01-11T21:41:19Z

📝 Description

Type: 🪲 bugfix

Added a missing import from IPython.display import display to resolve the undefined name error 'display' in custom_abundance.py

Closes #2941

📌 Resources

Examples, notebooks, and links to useful references.

🚦 Testing

How did you test these changes?

Testing pipeline
Other method (describe)
My changes can't be tested (explain why)

☑️ Checklist

I requested two reviewers for this pull request
I updated the documentation according to my changes
I built the documentation by applying the build_docs label

Note: If you are not allowed to perform any of these actions, ping (@) a contributor.

This reverts commit 55a190d.

tardis-bot · 2025-01-11T21:43:11Z

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

2	RET505	[ ] Unnecessary `else` after `return` statement
2	F811  	[ ] Redefinition of unused `input_d_time_0_eventhandler` from line 1714
2	UP032 	[*] Use f-string instead of `format` call
1	RET506	[ ] Unnecessary `else` after `raise` statement
1	SIM103	[*] Return the condition directly
1	I001  	[*] Import block is un-sorted or un-formatted
1	E712  	[*] Avoid equality comparisons to `True`; use `if obj.new:` for truth checks
1	UP031 	[*] Use format specifiers instead of percent format

Complete output(might be large):

tardis/visualization/widgets/custom_abundance.py:2:1: I001 [*] Import block is un-sorted or un-formatted
tardis/visualization/widgets/custom_abundance.py:33:60: F811 [*] Redefinition of unused `quantity_linspace` from line 17
tardis/visualization/widgets/custom_abundance.py:605:47: UP032 [*] Use f-string instead of `format` call
tardis/visualization/widgets/custom_abundance.py:738:9: SIM103 Return the condition directly
tardis/visualization/widgets/custom_abundance.py:742:9: RET505 Unnecessary `else` after `return` statement
tardis/visualization/widgets/custom_abundance.py:918:12: E712 Avoid equality comparisons to `True`; use `if obj.new:` for truth checks
tardis/visualization/widgets/custom_abundance.py:1131:9: RET505 Unnecessary `else` after `return` statement
tardis/visualization/widgets/custom_abundance.py:1422:9: RET506 Unnecessary `else` after `raise` statement
tardis/visualization/widgets/custom_abundance.py:1474:27: UP031 Use format specifiers instead of percent format
tardis/visualization/widgets/custom_abundance.py:1689:36: UP032 [*] Use f-string instead of `format` call
tardis/visualization/widgets/custom_abundance.py:1725:9: F811 Redefinition of unused `input_d_time_0_eventhandler` from line 1714
Found 11 errors.
[*] 4 fixable with the `--fix` option (3 hidden fixes can be enabled with the `--unsafe-fixes` option).

codecov · 2025-01-11T21:55:54Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 70.03%. Comparing base (2a06fdf) to head (0462b65).
Report is 2 commits behind head on master.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #2942      +/-   ##
==========================================
- Coverage   70.39%   70.03%   -0.36%     
==========================================
  Files         222      222              
  Lines       16157    16158       +1     
==========================================
- Hits        11373    11317      -56     
- Misses       4784     4841      +57

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

tardis-bot · 2025-01-11T22:36:40Z

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

Significantly changed benchmarks:

All benchmarks:

Benchmarks that have stayed the same:

| Change   | Before [5e2d0bb3] <master>   | After [0462b65c]    | Ratio   | Benchmark (Parameter)                                                                                                               |
|----------|------------------------------|---------------------|---------|-------------------------------------------------------------------------------------------------------------------------------------|
|          | 2.29±2μs                     | 3.72±3μs            | ~1.62   | transport_montecarlo_estimators_radfield_estimator_calcs.BenchmarkMontecarloMontecarloNumbaPacket.time_update_line_estimators       |
|          | 1.68±0.3μs                   | 1.97±0.5μs          | ~1.17   | transport_geometry_calculate_distances.BenchmarkTransportGeometryCalculateDistances.time_calculate_distance_line                    |
|          | 3.01±0ms                     | 2.73±0.01ms         | ~0.91   | opacities_opacity_state.BenchmarkOpacitiesOpacityState.time_opacity_state_initialize('scatter')                                     |
|          | 43.4±3μs                     | 39.3±0.02μs         | ~0.90   | transport_montecarlo_packet_trackers.BenchmarkTransportMontecarloPacketTrackers.time_generate_rpacket_tracker_list                  |
|          | 7.35±2μs                     | 6.58±2μs            | ~0.90   | transport_montecarlo_vpacket.BenchmarkMontecarloMontecarloNumbaVpacket.time_trace_vpacket_volley                                    |
|          | 25.1±7μs                     | 21.0±5μs            | ~0.84   | transport_montecarlo_packet_trackers.BenchmarkTransportMontecarloPacketTrackers.time_generate_rpacket_last_interaction_tracker_list |
|          | 50.4±30μs                    | 41.3±30μs           | ~0.82   | transport_montecarlo_interaction.BenchmarkTransportMontecarloInteraction.time_line_scatter                                          |
|          | 5.28±1μs                     | 3.23±0.5μs          | ~0.61   | transport_montecarlo_vpacket.BenchmarkMontecarloMontecarloNumbaVpacket.time_trace_bad_vpacket                                       |
|          | 551±200ns                    | 571±200ns           | 1.04    | opacities_opacity.BenchmarkMontecarloMontecarloNumbaOpacities.time_pair_creation_opacity_calculation                                |
|          | 572±100ns                    | 591±400ns           | 1.03    | opacities_opacity.BenchmarkMontecarloMontecarloNumbaOpacities.time_compton_opacity_calculation                                      |
|          | 38.6±0s                      | 38.8±0s             | 1.00    | run_tardis.BenchmarkRunTardis.time_run_tardis                                                                                       |
|          | 2.08±0m                      | 2.08±0m             | 1.00    | spectrum_formal_integral.BenchmarkTransportMontecarloFormalIntegral.time_FormalIntegrator_functions                                 |
|          | 1.20±0μs                     | 1.20±0μs            | 1.00    | transport_geometry_calculate_distances.BenchmarkTransportGeometryCalculateDistances.time_calculate_distance_boundary                |
|          | 66.5±0.2ms                   | 66.4±0.3ms          | 1.00    | transport_montecarlo_packet_trackers.BenchmarkTransportMontecarloPacketTrackers.time_rpacket_trackers_to_dataframe                  |
|          | 2.58±0.4ms                   | 2.59±0.4ms          | 1.00    | transport_montecarlo_single_packet_loop.BenchmarkTransportMontecarloSinglePacketLoop.time_single_packet_loop                        |
|          | 1.06±0m                      | 1.06±0m             | 0.99    | run_tardis.BenchmarkRunTardis.time_run_tardis_rpacket_tracking                                                                      |
|          | 45.5±20μs                    | 45.0±20μs           | 0.99    | transport_montecarlo_interaction.BenchmarkTransportMontecarloInteraction.time_line_emission                                         |
|          | 1.75±0.02ms                  | 1.73±0.02ms         | 0.99    | transport_montecarlo_main_loop.BenchmarkTransportMontecarloMontecarloMainLoop.time_montecarlo_main_loop                             |
|          | 759±1ns                      | 725±0.2ns           | 0.96    | transport_montecarlo_interaction.BenchmarkTransportMontecarloInteraction.time_thomson_scatter                                       |
|          | 4.40±0.3ms                   | 4.13±0.06ms         | 0.94    | opacities_opacity_state.BenchmarkOpacitiesOpacityState.time_opacity_state_initialize('macroatom')                                   |
|          | 7.20±0.6μs                   | 6.67±0.6μs          | 0.93    | transport_montecarlo_vpacket.BenchmarkMontecarloMontecarloNumbaVpacket.time_trace_vpacket                                           |
|          | 3.54±0.5μs                   | 3.26±0.5μs          | 0.92    | transport_montecarlo_vpacket.BenchmarkMontecarloMontecarloNumbaVpacket.time_trace_vpacket_within_shell                              |
|          | 581±200ns                    | 531±200ns           | 0.91    | opacities_opacity.BenchmarkMontecarloMontecarloNumbaOpacities.time_photoabsorption_opacity_calculation                              |
|          | 226±0.06ns                   | 206±0.1ns           | 0.91    | spectrum_formal_integral.BenchmarkTransportMontecarloFormalIntegral.time_intensity_black_body                                       |

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

Sonu0305 · 2025-01-11T23:01:24Z

Hi @atharva-2001 @andrewfullard @jvshields ,
Could you please review when you have a chance, Thank You.

andrewfullard · 2025-01-13T15:52:28Z

Why is this the correct fix? There is also a display method in the class.

This reverts commit dda572c, reversing changes made to 9cf38cf.

This reverts commit dda572c, reversing changes made to b4bc6ac.

review-notebook-app · 2025-01-13T19:57:35Z

Check out this pull request on

See visual diffs & provide feedback on Jupyter Notebooks.

Powered by ReviewNB

This reverts commit 19591f5.

Sonu0305 · 2025-01-13T20:06:36Z

Hi @andrewfullard ,
The tests that are failing seem to be unrelated to the changes that i made, what should I do?
Two tests are failing not continuum ubuntu-latest and not continuum macos-latest.
I realised that this is I think because of #2800 being worked upon, it will all pass once it is merged, Thank You.

Sonu0305 · 2025-01-13T20:09:15Z

Why is this the correct fix? There is also a display method in the class.

Also, yes, I think this is the correct fix.
Yes, I already noticed the display method defined at the end of the class DensityEditor which seems out of scope of the dpd_dtype_eventhandler method so...
Also it is ensured that output is cleared everytime before any other displays, this was already written just before dpd_dtype_eventhandler is defined

Sonu0305 · 2025-01-14T12:24:41Z

Hi @atharva-2001 @KasukabeDefenceForce @andrewfullard ,
The tests are successfully passing now, please review when you have a chance, Thank You.

andrewfullard · 2025-01-14T21:05:11Z

Hello! Are you planning to apply to Google Summer of Code?

Sonu0305 · 2025-01-15T01:11:53Z

Hello! Are you planning to apply to Google Summer of Code?

@andrewfullard
Yes, I am going to apply for Gsoc. I would love to continue contributing to Tardis, Thank You

andrewfullard · 2025-01-15T14:15:41Z

Hello! Are you planning to apply to Google Summer of Code?

@andrewfullard Yes, I am going to apply for Gsoc. I would love to continue contributing to Tardis, Thank You

Great! I think we have seen enough of your capabilities with the number of contributions you have made. Please respond to reviews and comments on your existing PRs before writing more. Thanks!

Sonu0305 · 2025-01-15T18:17:42Z

Great! I think we have seen enough of your capabilities with the number of contributions you have made. Please respond to reviews and comments on your existing PRs before writing more. Thanks!

Thank you so much for your kind words and for recognizing my contributions.
Yes, I have already addressed the requested changes and either incorporated them or provided comments to ensure that no tests are failing and all necessary modifications are included. I am now awaiting further feedback and reviews,
Excited to continue contributing!

andrewfullard · 2025-01-21T16:44:59Z

Is there a way you could demonstrate what errors this fixes? Aside from linting errors in an IDE.

Sonu0305 · 2025-01-21T18:32:25Z

Is there a way you could demonstrate what errors this fixes? Aside from linting errors in an IDE.

The change specifically fixes the undefined display( ) used.

andrewfullard · 2025-01-21T19:34:26Z

Is there a way you could demonstrate what errors this fixes? Aside from linting errors in an IDE.

The change specifically fixes the undefined display( ) used.

Yes, and what impact does that have on running the code? Since our documentation and tests work using part of this code, what is the error that is produced as part of normal running?

Sonu0305 added 5 commits January 2, 2025 23:26

for test

55a190d

Merge branch 'tardis-sn:master' into master

fee6b5f

Merge branch 'tardis-sn:master' into master

b6ce897

Revert "for test"

6d3b92c

This reverts commit 55a190d.

Added import from IPython.display

9cf38cf

wkerzendorf requested review from KasukabeDefenceForce and atharva-2001 January 13, 2025 15:48

andrewfullard added the visualization label Jan 13, 2025

Sonu0305 added 3 commits January 14, 2025 00:55

Merge branch 'tardis-sn:master' into fix#2941

dda572c

Revert "Merge branch 'tardis-sn:master' into fix#2941"

19591f5

This reverts commit dda572c, reversing changes made to 9cf38cf.

Revert "Merge branch 'tardis-sn:master' into fix#2941"

b0e53c7

This reverts commit dda572c, reversing changes made to b4bc6ac.

Sonu0305 added 2 commits January 14, 2025 01:29

added a missing import

6f7c604

Reapply "Merge branch 'tardis-sn:master' into fix#2941"

28b2ad7

This reverts commit 19591f5.

Merge branch 'tardis-sn:master' into fix#2941

0462b65

andrewfullard added the GSoC 🌞 label Jan 15, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Adds a missing import in custom_abundance.py #2942

Adds a missing import in custom_abundance.py #2942

Sonu0305 commented Jan 11, 2025

tardis-bot commented Jan 11, 2025 •

edited

Loading

codecov bot commented Jan 11, 2025 •

edited

Loading

tardis-bot commented Jan 11, 2025 •

edited

Loading

Sonu0305 commented Jan 11, 2025

andrewfullard commented Jan 13, 2025

review-notebook-app bot commented Jan 13, 2025

Sonu0305 commented Jan 13, 2025 •

edited

Loading

Sonu0305 commented Jan 13, 2025 •

edited

Loading

Sonu0305 commented Jan 14, 2025 •

edited

Loading

andrewfullard commented Jan 14, 2025

Sonu0305 commented Jan 15, 2025 •

edited

Loading

andrewfullard commented Jan 15, 2025

Sonu0305 commented Jan 15, 2025

andrewfullard commented Jan 21, 2025

Sonu0305 commented Jan 21, 2025

andrewfullard commented Jan 21, 2025

Adds a missing import in custom_abundance.py #2942

Are you sure you want to change the base?

Adds a missing import in custom_abundance.py #2942

Conversation

Sonu0305 commented Jan 11, 2025

📝 Description

📌 Resources

🚦 Testing

☑️ Checklist

tardis-bot commented Jan 11, 2025 • edited Loading

codecov bot commented Jan 11, 2025 • edited Loading

Codecov Report

tardis-bot commented Jan 11, 2025 • edited Loading

Sonu0305 commented Jan 11, 2025

andrewfullard commented Jan 13, 2025

review-notebook-app bot commented Jan 13, 2025

Sonu0305 commented Jan 13, 2025 • edited Loading

Sonu0305 commented Jan 13, 2025 • edited Loading

Sonu0305 commented Jan 14, 2025 • edited Loading

andrewfullard commented Jan 14, 2025

Sonu0305 commented Jan 15, 2025 • edited Loading

andrewfullard commented Jan 15, 2025

Sonu0305 commented Jan 15, 2025

andrewfullard commented Jan 21, 2025

Sonu0305 commented Jan 21, 2025

andrewfullard commented Jan 21, 2025

tardis-bot commented Jan 11, 2025 •

edited

Loading

codecov bot commented Jan 11, 2025 •

edited

Loading

tardis-bot commented Jan 11, 2025 •

edited

Loading

Sonu0305 commented Jan 13, 2025 •

edited

Loading

Sonu0305 commented Jan 13, 2025 •

edited

Loading

Sonu0305 commented Jan 14, 2025 •

edited

Loading

Sonu0305 commented Jan 15, 2025 •

edited

Loading