parsotat / BatAnalysis

A python HEASOFT wrapper for processing Swift-BAT data.
MIT License
28 stars 11 forks source link

Reformatted with `black`. Changed np.int (deprecated) to int. #14

Closed dmopalmer closed 10 months ago

dmopalmer commented 10 months ago

Cleaned up the code by:

  1. Reformatting the .py files with black.
  2. Replacing np.int (which is deprecated) with int

The .ipynb Jupyter Notebook files were not reformatted.

This is to address #13 and a secondary aspect of #12.

parsotat commented 10 months ago

There's a lot of changes in one commit here. For ease of review can you break this formatting change into different commits where black is run on one file at a time and then of course a final commit where np.int is converted to int?

dmopalmer commented 10 months ago

I can do that, but I don't know how much it will ease review. Do you want me to push each time or only at the end.

parsotat commented 10 months ago

pushing each time would be great so I can just go through the files as you get them done

dmopalmer commented 10 months ago

I just did 5, an dpushed, so I will push after each one from now on.

dmopalmer commented 10 months ago

Should I skip black on the .py files in the notebook/ directory, to reduce divergence with the corresponding .ipynb files?

parsotat commented 10 months ago

I would say that we should format all the .py files, including those in the notebook directory

dmopalmer commented 10 months ago

YOu pulled while I was still pushing.

(Correction: It appears that I was the one who causes the PR to close. I re-opened.)

Should I also reformat the .ipynb files?

parsotat commented 10 months ago

with the exception of the 3 comments on the example files the other reformats look good to me

parsotat commented 10 months ago

I think that looks great to me. Thanks David. On Nov 13, 2023, at 6:17 PM, David Palmer @.***> wrote: @dmopalmer commented on this pull request.

In notebooks/trial_NGC2992.py:

table_everything = ba.from_heasarc(**queryargs)

-minexposure = 1000 # cm^2 after cos adjust -exposures = np.array([object_batsource.exposure(ra=row['RA'], dec=row['DEC'], roll=row['ROLL_ANGLE'])[0] for row in table_everything]) +minexposure = 1000 # cm^2 after cos adjust +exposures = np.array(

What do you think of the format exposures = np.array( [object_batsource.exposure(ra=row["RA"], dec=row["DEC"], roll=row["ROLL_ANGLE"])[0] for row in table_everything ])

? (For all three files). I think it improves the clarity, even if it lacks the purity of letting black format everything.

—Reply to this email directly, view it on GitHub, or unsubscribe.You are receiving this because you were assigned.Message ID: @.***>

dmopalmer commented 10 months ago

Last push with those reformat. Ready to go.

parsotat commented 10 months ago

This looks good to me. I will merge now.