Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

i.atcorr: major refactoring of create_iwave.py #3886

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

pesekon2
Copy link
Contributor

  • speed up (e.g., do not run loops if one run is sufficient)
  • lower the amount of code duplicity
  • remove lines that actually do not do anything (e.g., lines containing declaration print without brackets)
  • general beauty salon treatment

* speed up (e.g., do not run loops if one run is sufficient)

* lower the amount of code duplicity

* remove lines that actually do not do anything (e.g., lines containing declaration print without brackets)

* general beauty salon treatment
@pesekon2 pesekon2 added Python Related code is in Python imagery labels Jun 18, 2024
@pesekon2 pesekon2 self-assigned this Jun 18, 2024
imagery/i.atcorr/create_iwave.py Show resolved Hide resolved
imagery/i.atcorr/create_iwave.py Outdated Show resolved Hide resolved
imagery/i.atcorr/create_iwave.py Outdated Show resolved Hide resolved
Copy link
Member

@echoix echoix left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Didn't have time to finish reading all files, but here's a start

bands[-1] = bands[-1].strip()
print(" > Number of bands found: %d" % len(bands))
infile.close()
print(f" > Number of bands found: {len(bands)}")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A linter should complain that bands could be undefined here.

for i in range(len(filter_f) + 1):
if i % 8 is 0:
if i is not 0:
if i % 8 == 0:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a infinitesimal implementation-dependent optimisation possible here: I stumbled upon something on stack overflow this weekend, not exactly related to this, but the example had this form.

This pattern uses at least 2 more opcodes than another possibility, as it has to protect himself from the variable not being an integer when comparing to 0. There's an alternative, but it was open three days ago on my computer, and I'm writing on my phone. It had something to do with either using "in" or nothing at all.

I'm just nerdsniping here, if you are curious to microoptimise here.

pstring += value_wo_leading_zero
if i > 1 and i < len(filter_f):
pstring += ", "
if i is not 1:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The replacement of this doesn't seem exactly the equivalent. Is the new logic expected behavior? (Old one got triggered for -1, 0, 2, 3, 4..., new gets triggered for 2, 3, 4...)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
imagery module Python Related code is in Python
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants