config_checker update and linting for C408,C419,C405, C409, E721,F401,E722 #560

alex-rakowski · 2023-11-10T05:00:57Z

Getting rid of some of the bare exceptions. Adding the correct Error if I could work it out otherwise using Exception.

merging in more ruff errors

alex-rakowski · 2023-11-10T10:07:42Z

Added more minor changes:

sezelt

The majority of these changes are good, but at least one is wrong, and some are behavior changes that should be evaluated.
It would also be nice to clean up the TODOs before merging.

py4DSTEM/braggvectors/diskdetection_aiml.py

py4DSTEM/io/filereaders/read_K2.py

py4DSTEM/io/filereaders/read_arina.py

py4DSTEM/process/utils/utils.py

py4DSTEM/utils/configuration_checker.py

py4DSTEM/datacube/virtualimage.py

bsavitzky

Looks good - let us know when you're done modifying that import_spec --> try...except statements and I think we can merge

bsavitzky · 2023-11-14T16:31:05Z

@sezelt I'm good with these edits but will wait for your approval as well.

sezelt

It seems there are at least 7 remaining TODOs added by this PR. I'd prefer not to add too many of these, so if there are cases where it's possible to either deduce or test what errors should be captured we should do that. I also marked a few minor comments that would be good to clean up.

sezelt · 2023-11-16T23:43:17Z

py4DSTEM/braggvectors/diskdetection_aiml.py

@@ -840,7 +837,8 @@ def _integrate_disks(DP, maxima_x, maxima_y, maxima_int, int_window_radius=1):
        disks.append(np.average(disk))
    try:
        disks = disks / max(disks)
-    except:
+    # TODO work out what exception would go here


I'd guess this is a ZeroDivisionError, but I didn't want to guess I can remove the TODO if prefered.

py4DSTEM/braggvectors/diskdetection_aiml.py

sezelt · 2023-11-16T23:45:30Z

py4DSTEM/io/filereaders/read_K2.py

@@ -124,7 +124,8 @@ def __init__(
            # this may be wrong for binned data... in which case the reader doesn't work anyway!
            Q_Nx = gtg.allTags[".SI Image Tags.Acquisition.Parameters.Detector.height"]
            Q_Ny = gtg.allTags[".SI Image Tags.Acquisition.Parameters.Detector.width"]
-        except:
+        # TODO check this is the correct error type


I can remove as Exception will handle what we want, but I think it would be better to add the expected exception/s in the future, but I'm not familiar enough to check/test

sezelt · 2023-11-16T23:46:03Z

py4DSTEM/io/legacy/read_utils.py

@@ -100,7 +97,8 @@ def get_N_dataobjects(filepath, topgroup="4DSTEM_experiment"):
        N_pla = len(f[topgroup]["data/pointlistarrays"].keys())
        try:
            N_coords = len(f[topgroup]["data/coordinates"].keys())
-        except:
+        # TODO work out what exception will be raised ValueError, AttributeError, BS thinks KeyError


I can remove

Actually, I'm going to leave this as a TODO I don't want to break this and I'm not familiar enough with this part to test

py4DSTEM/utils/configuration_checker.py

…t-fixes

alex-rakowski · 2023-11-17T21:55:39Z

@sezelt, I've resolved most of the changes you requested. I would prefer to leave the remaining TODOs as I think we should address them next time they're worked on, but I don't have enough familiarity with the code to be confident in guessing or being able to test.

bsavitzky · 2023-11-21T21:30:19Z

Hey @sezelt @alex-rakowski -

This is a large set of small edits which will affect the linting PR and likely modifies many similar pieces of code. I think it's best to finish / resolve this PR first, then rebase PR#563 once this is merged, before continuing to work on that PR. Otherwise we're going to run into a lot of conflicts and work duplication.

Could you please also make a checklist of the kinds modifications made in this PR? (E.g. modifying exceptions, etc). I'd also request modifying the PR name.

alex-rakowski · 2023-11-21T22:31:54Z

This PR does the following:

Updates config_checker so that it does scrapes the required modules and optional installs from setup.py and adds generic version checking for installed versions of packages. New behavior is if we add a dependency or add a new optional set of dependencies e.g. ~"dev" : ["pytest", "black", "flake8"], it will no automatically pick up on "dev" install optional and test whether all the dependencies are met, and report the state i.e. all met or not. Previously, we had to manually update by adding a dependency or optional dependencies set to a list or dict.

In addition, it lints for the following errors:

Added more minor changes:

C408 - unnecessary-collection-call

C419 - unnecessary-comprehension-any-all

unnecessary-literal-set (C405)

unnecessary-literal-within-tuple-call (C409)

E721 - type-comparison

F401 - unused-import

E722 - bare-except

sezelt · 2024-03-08T22:17:53Z

I've tried to sync this with dev and merge with #571

sezelt · 2024-03-08T22:20:23Z

It seems to not pass its own tests anymore, though, so something is off with the updated flake8 config...

alex-rakowski added 19 commits November 9, 2023 11:12

initial bare exception fix

53e8796

correcting multiple exception handling

fbbc232

Changing to Exception if unsure

2212461

changing to assertion error

51f078f

removing TODO message

db59746

Changing to a Exception

be609b3

changing from ValueError to Exception

08957ff

black

e6b6ade

ruff C408

857715a

ruff --select C419

7892191

C409 C405

4288ca2

trailing commas

1a00dd1

black

dfa7546

E721

79b2559

F401 - unused imports

0b20634

black

d3ccf03

more E721

9022bee

Merge branch 'bare_exceptions' into unnecessary-collection-calls

83a727a

Merge pull request #16 from alex-rakowski/unnecessary-collection-calls

15f387f

merging in more ruff errors

alex-rakowski changed the title ~~Clearing up some bare exceptions~~ Clearing up minor issues Nov 10, 2023

sezelt requested changes Nov 10, 2023

View reviewed changes

alex-rakowski added 8 commits November 10, 2023 13:29

removing old comment

dd1a74b

adding KeyError

9657c42

adding KeyError to correct place ...

dbfa1b2

adding hdf5plugin back in

6b5dfec

Changing to Exception to catch more errors

89ad26a

changning cupy GPU count method

ed34a99

black

5275729

using importlib to populate requirements

96c96ae

bsavitzky approved these changes Nov 13, 2023

View reviewed changes

alex-rakowski added 2 commits November 13, 2023 16:46

changing find_spec to try except

03e9e61

adding suspected KeyError to TODO

436d1e4

F811, F523/F524, F841, F741 - Corrections

5f5d895

sezelt requested changes Nov 16, 2023

View reviewed changes

alex-rakowski added 11 commits November 16, 2023 17:02

updating to autoscrape optional depends

98c21d2

fixing typo in docstring

0f9b096

removing unused vars

dc9c632

Merge branch 'dev' of github.com:py4dstem/py4DSTEM into crystal-forma…

4936f9e

…t-fixes

adding generic version extra tests

03986fa

removing TODO

8e4f7c9

ModuleNotFoundErrors

0a77665

removing TODOS

92c6942

changing logic for print(Running Extra Checks run)

c280c8e

cleaning up old commented out code

f1a9b3a

remove unused imports

ffa70f9

fixing import test

0b10f48

alex-rakowski changed the title ~~Clearing up minor issues~~ config_checker update and linting for C408,C419,C405, C409, E721,F401,E722 Nov 21, 2023

alex-rakowski mentioned this pull request Jan 3, 2024

Update check_config workflow #596

Merged

sezelt added 4 commits March 8, 2024 17:03

Merge remote-tracking branch 'upstream/dev' into bare_exceptions

83453df

missing import

19914f6

other missing import

9397208

Merge branch 'crystal-format-fixes' into bare_exceptions

9201154

sezelt mentioned this pull request Mar 8, 2024

F811, F523/F524, F841, F741 - Corrections in crystal.py #571

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

config_checker update and linting for C408,C419,C405, C409, E721,F401,E722 #560

config_checker update and linting for C408,C419,C405, C409, E721,F401,E722 #560

alex-rakowski commented Nov 10, 2023

alex-rakowski commented Nov 10, 2023 •

edited

Loading

sezelt left a comment

bsavitzky left a comment •

edited

Loading

bsavitzky commented Nov 14, 2023

sezelt left a comment

sezelt Nov 16, 2023

alex-rakowski Nov 17, 2023

sezelt Nov 16, 2023

alex-rakowski Nov 17, 2023

sezelt Nov 16, 2023

alex-rakowski Nov 17, 2023

alex-rakowski Nov 17, 2023

alex-rakowski commented Nov 17, 2023

bsavitzky commented Nov 21, 2023

alex-rakowski commented Nov 21, 2023

sezelt commented Mar 8, 2024

sezelt commented Mar 8, 2024

config_checker update and linting for C408,C419,C405, C409, E721,F401,E722 #560

Are you sure you want to change the base?

config_checker update and linting for C408,C419,C405, C409, E721,F401,E722 #560

Conversation

alex-rakowski commented Nov 10, 2023

alex-rakowski commented Nov 10, 2023 • edited Loading

sezelt left a comment

Choose a reason for hiding this comment

bsavitzky left a comment • edited Loading

Choose a reason for hiding this comment

bsavitzky commented Nov 14, 2023

sezelt left a comment

Choose a reason for hiding this comment

sezelt Nov 16, 2023

Choose a reason for hiding this comment

alex-rakowski Nov 17, 2023

Choose a reason for hiding this comment

sezelt Nov 16, 2023

Choose a reason for hiding this comment

alex-rakowski Nov 17, 2023

Choose a reason for hiding this comment

sezelt Nov 16, 2023

Choose a reason for hiding this comment

alex-rakowski Nov 17, 2023

Choose a reason for hiding this comment

alex-rakowski Nov 17, 2023

Choose a reason for hiding this comment

alex-rakowski commented Nov 17, 2023

bsavitzky commented Nov 21, 2023

alex-rakowski commented Nov 21, 2023

sezelt commented Mar 8, 2024

sezelt commented Mar 8, 2024

alex-rakowski commented Nov 10, 2023 •

edited

Loading

bsavitzky left a comment •

edited

Loading