Skip to content

A few updates I needed to make to get this running#23

Open
ddrewpublic wants to merge 3 commits intoZyin055:mainfrom
ddrewpublic:main
Open

A few updates I needed to make to get this running#23
ddrewpublic wants to merge 3 commits intoZyin055:mainfrom
ddrewpublic:main

Conversation

@ddrewpublic
Copy link
Copy Markdown

Hi - I needed to make a few updates to this to get it running (which it did - thanks!)

Changes made:

  • type hints (Tuple, instead of tuple, Dict instead of dict etc...) using typing_extensions (4.7.1, in case that make a difference).
  • all the sys.exit(e) to sys.exit(str(e))
  • Then a bunch of formatting changes to make the code PEP compliant, as my IDE was complaining about it.

Thanks!

@Zyin055
Copy link
Copy Markdown
Owner

Zyin055 commented Dec 18, 2023

It looks like all the changes are for formatting and doesn't fix any bugs, correct?

I haven't used this code since the last update 10 months ago. I'm hesitant to accept a PR that doesn't fix a any bugs since I don't feel like testing these changes myself.

@ddrewpublic
Copy link
Copy Markdown
Author

Hi - totally understandable!

There was one bug / sticking point that got me started :

23 - : GRAPH_IMAGE_SIZE: tuple[int, int] = (19, 9) # (X,Y) tuple in inches (multiply by 100 for (X,Y) pixel size for the output graphs)

25 + : (X,Y) tuple in inches (multiply by 100 for (X,Y) pixel size for the output graphs)
26 + : GRAPH_IMAGE_SIZE: Tuple[int, int] = (19, 9)

The lower case t meant that I was getting a TypeError: 'type' object is not subscriptable.

I'm not completely familiar with how python declares datatypes, but I have used the typing package

Once I imported that, there were a load more that needed updating.

Mostly dict to Dict, but also a missing type in line 417 -

  • learn_rate_changes: dict[int, (int, float)]
  • learn_rate_changes: Dict[int, Union[int, float]]

The formatting is because I can't leave stuff alone!

@Zyin055
Copy link
Copy Markdown
Owner

Zyin055 commented Dec 18, 2023

The typing issue is related to #6

If I recall correctly, it had something to do with Python changing how their typing worked in some version, don't recall if it was before or after v3.10 (the version that A1111 uses). But the script works fine with 3.10.

@ddrewpublic
Copy link
Copy Markdown
Author

ddrewpublic commented Dec 18, 2023

Ah - that could be it. I'm running 3.8.10 system wide, A1111 running in a docker instance that has 3.10.

Sorry!

@xeghesoopn
Copy link
Copy Markdown

this sorted my issues with this script, thanks! no pytorch for python 3.12 yet, and was getting the "TypeError: 'type' object is not subscriptable" with a version before python 3.10. pull request clears it up, dropping a requirements.txt would also clear it up

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants