Changed to fstring #1706

svenkat19 · 2021-07-12T16:58:16Z

Description
<Changed strings to fstrings in python 3>

Motivation and context

How has this been tested?

Testing pipeline.
[ x] Other.

Type of change

Bug fix.
New feature.
Breaking change.
[x ] None of the above.

Checklist

My change requires a change to the documentation.
- I have updated the documentation accordingly.
- (optional) I have built the documentation on my fork following the instructions.
I have assigned and requested two reviewers for this pull request.

tardis-bot · 2021-07-12T17:04:18Z

Before a pull request is accepted, it must meet the following criteria:

Is the necessary information provided?
Is this a duplicate PR?
- If a new PR is clearly a duplicate, ask how this PR is different from the original PR?
- If this PR is about to be merged, close the original PR with a link to this new PR that solved the issue.
Does it pass existing tests and are new tests provided if required?
- The test coverage should not decrease, and for new features should be close to 100%.
Is the code tidy?
- No unnecessary print lines or code comments.

codecov · 2021-07-12T17:09:34Z

Codecov Report

Merging #1706 (ae4e954) into master (7d3899e) will increase coverage by 0.81%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master    #1706      +/-   ##
==========================================
+ Coverage   61.06%   61.87%   +0.81%     
==========================================
  Files          63       63              
  Lines        5820     5852      +32     
==========================================
+ Hits         3554     3621      +67     
+ Misses       2266     2231      -35

Impacted Files	Coverage Δ
tardis/tardis/io/atom_data/base.py	`89.77% <0.00%> (-1.51%)`	⬇️
tardis/tardis/util/base.py	`74.35% <0.00%> (-1.44%)`	⬇️
tardis/tardis/base.py	`57.69% <0.00%> (-1.40%)`	⬇️
...s/tardis/plasma/properties/radiative_properties.py	`77.18% <0.00%> (-0.53%)`	⬇️
tardis/tardis/io/util.py	`71.67% <0.00%> (-0.52%)`	⬇️
tardis/tardis/simulation/base.py	`82.88% <0.00%> (-0.45%)`	⬇️
tardis/tardis/plasma/properties/nlte.py	`38.83% <0.00%> (-0.39%)`	⬇️
tardis/tardis/analysis.py	`30.16% <0.00%> (ø)`
tardis/tardis/montecarlo/base.py	`88.77% <0.00%> (ø)`
tardis/tardis/util/colored_logger.py
... and 7 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7d3899e...ae4e954. Read the comment docs.

wkerzendorf · 2021-07-12T20:18:10Z

tardis/analysis.py

-                    self.current_no_packets,
-                    self.last_line_list_in.ix[event.ind],
-                )
+                f"Line_in ({len(event.ind)}/{self.current_no_packets}):\n{self.last_line_list_in.ix[event.ind]}"


please be PEP8 compliant

DhruvSondhi

Thank you @svenkat19, for the Pull Request. I have requested some changes else everything seems to be on the right track.

DhruvSondhi · 2021-07-13T05:56:04Z

tardis/gui/widgets.py

-                        "wavelength"
-                    ],
-                )
+                f"{key[0]}-{key[1]} {self.parent.model.atom_data.lines.ix[value[0]]["wavelength"]}"


The appended statement here has been changed. Please check that the statements adhere to their original intended meaning.

Suggested change

f"{key[0]}-{key[1]} {self.parent.model.atom_data.lines.ix[value[0]]["wavelength"]}"

f"{key[0]}-{key[1]} {self.parent.model.atom_data.lines.ix[value[0]]["wavelength"]:.2f} A"

DhruvSondhi · 2021-07-13T05:56:48Z

tardis/gui/widgets.py

@@ -1225,7 +1214,7 @@ def on_species_clicked(self, index):
        last_line_in_model = self.createTable(
            [
                last_line_in_string,
-                ["Num. pkts %d" % current_last_line_in.wavelength.count()],
+                [f"Num. pkts {current_last_line_in.wavelength.count()}" ],


Extra space after the ending " can be removed.

DhruvSondhi · 2021-07-13T05:57:52Z

tardis/model/base.py

-                    "The following columns are specified in the csvy"
-                    "model file, but are IGNORED by TARDIS: %s"
-                    % (str(unsupported_columns))
+     f"The following columns are specified in the csvy model file, but are IGNORED by TARDIS: {str(unsupported_columns)}"


Please adhere to the Black formatting styling. This seems to be off here in terms of the formatting of the message under the logger.warning

svenkat19 · 2021-07-13T09:28:59Z

Thank you @svenkat19, for the Pull Request. I have requested some changes else everything seems to be on the right track.

Could you please go through and let me know if I must change anything else? Cheers :)

DhruvSondhi

Please check these comments. Otherwise, it looks Good to me 🚀.

DhruvSondhi · 2021-07-13T14:19:16Z

tardis/analysis.py

@@ -262,7 +254,7 @@ def load_t_inner(self, iterations=None):

        for iter in iterations:
            t_inners.append(
-                hdf_store["model%03d/configuration" % iter].ix["t_inner"]
+                hdf_store[f"model{iter:003d}/configuration"].ix["t_inner"]


Why has there been a change in the string formatting? What I mean is why is it 003d instead of 03d?

Why has there been a change in the string formatting? What I mean is why is it 003d instead of 03d?

This has got to be the silliest error done by me😅. Do let me know if there are any mistakes I must fix. Cheers !!

Sure, Everything else seems to be Awesome. These things can be overlooked easily that's why I raised it here. Thank you for making the required changes 😄

DhruvSondhi · 2021-07-13T14:19:32Z

tardis/analysis.py

@@ -281,8 +273,8 @@ def load_t_rads(self, iterations=None):
            iterations = self.iterations[iterations]

        for iter in iterations:
-            current_iter = "iter%03d" % iter
-            t_rads_dict[current_iter] = hdf_store["model%03d/t_rads" % iter]
+            current_iter = f"iter{iter:003d}" 


Check the previous comment.

DhruvSondhi · 2021-07-13T14:19:49Z

tardis/analysis.py

@@ -319,7 +311,7 @@ def load_level_populations(self, iterations=None):
            iterations = self.iterations[iterations]

        for iter in iterations:
-            current_iter = "iter%03d" % iter
+            current_iter = f"iter{iter:003d}" 


Same here 🤔

DhruvSondhi · 2021-07-13T14:20:01Z

tardis/analysis.py

@@ -381,7 +373,7 @@ def load_spectrum(self, iteration, spectrum_keyword="luminosity_density"):
        hdf_store = pd.HDFStore(self.hdf5_fname, "r")

        spectrum = hdf_store[
-            "model%03d/%s" % (self.iterations[iteration], spectrum_keyword)
+            f"model{self.iterations[iteration]:003d}/{spectrum_keyword}"


DhruvSondhi · 2021-07-13T14:20:11Z

tardis/analysis.py

@@ -426,7 +418,7 @@ def get_last_line_interaction(self, iteration=-1):
        self.load_atom_data()

        hdf_store = pd.HDFStore(self.hdf5_fname, "r")
-        model_string = "model" + ("%03d" % iteration) + "/%s"
+        model_string = "model" + (f"{iteration:003d}" ) + "/%s"


DhruvSondhi · 2021-07-13T16:43:22Z

Please do fix the tests that are failing. If you have any problems regarding the same, please do let us know 😄

wkerzendorf

please fix the PEP8 violations

Changed to fstring

8e33127

andrewfullard requested review from DhruvSondhi, andrewfullard and Rodot- July 12, 2021 17:04

andrewfullard added the codestyle label Jul 12, 2021

andrewfullard linked an issue Jul 12, 2021 that may be closed by this pull request

Convert Old Python 2 String formatting into Python 3 #1705

Closed

wkerzendorf reviewed Jul 12, 2021

View reviewed changes

DhruvSondhi suggested changes Jul 13, 2021

View reviewed changes

Updated strings

d522ab1

DhruvSondhi suggested changes Jul 13, 2021

View reviewed changes

003 typo fixed

aec2e51

DhruvSondhi approved these changes Jul 13, 2021

View reviewed changes

fixed error

87c198c

DhruvSondhi requested a review from wkerzendorf July 13, 2021 17:01

wkerzendorf reviewed Jul 14, 2021

View reviewed changes

fixed PEP8 violation in line 195

ae4e954

wkerzendorf merged commit 59c7f87 into tardis-sn:master Dec 8, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Changed to fstring #1706

Changed to fstring #1706

svenkat19 commented Jul 12, 2021

tardis-bot commented Jul 12, 2021

codecov bot commented Jul 12, 2021 •

edited

Loading

wkerzendorf Jul 12, 2021

DhruvSondhi left a comment

DhruvSondhi Jul 13, 2021

DhruvSondhi Jul 13, 2021

DhruvSondhi Jul 13, 2021

svenkat19 commented Jul 13, 2021

DhruvSondhi left a comment

DhruvSondhi Jul 13, 2021

svenkat19 Jul 13, 2021

DhruvSondhi Jul 13, 2021

DhruvSondhi Jul 13, 2021

DhruvSondhi Jul 13, 2021

DhruvSondhi Jul 13, 2021

DhruvSondhi Jul 13, 2021

DhruvSondhi commented Jul 13, 2021

wkerzendorf left a comment

	f"{key[0]}-{key[1]} {self.parent.model.atom_data.lines.ix[value[0]]["wavelength"]}"
	f"{key[0]}-{key[1]} {self.parent.model.atom_data.lines.ix[value[0]]["wavelength"]:.2f} A"

Changed to fstring #1706

Changed to fstring #1706

Conversation

svenkat19 commented Jul 12, 2021

tardis-bot commented Jul 12, 2021

codecov bot commented Jul 12, 2021 • edited Loading

Codecov Report

Choose a reason for hiding this comment

DhruvSondhi left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

svenkat19 commented Jul 13, 2021

DhruvSondhi left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

DhruvSondhi commented Jul 13, 2021

wkerzendorf left a comment

Choose a reason for hiding this comment

codecov bot commented Jul 12, 2021 •

edited

Loading