-
Notifications
You must be signed in to change notification settings - Fork 38
Flexible Graph configurations #46
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
Conversation
Zlopez
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks good, but I would like to separate the version bump to different PR.
|
Also the tests are failing. |
I am not sure if I can fix this one myself, because it seems like these are problems with repo actions: https://github.com/fschulze/sqlalchemy_schemadisplay/actions/runs/17805776025/job/50672153234?pr=46 Pre-commit and test steps seem to be using the updated actions/cache version, but somehow the action itself understands it is deprecated. Never mind! The message is misleading.... it should be bumped to 4.0.0 only. I'll look into it. |
.github/workflows/cd.yml
Outdated
| - name: Cache python dependencies | ||
| id: cache-pip | ||
| uses: actions/cache@v3.2.4 | ||
| uses: actions/cache@v4.0.5 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
From https://github.com/actions/toolkit/blob/main/packages/cache/RELEASES.md#400:
The new service will gradually roll out as of February 1st, 2025. The legacy service will also be sunset on the same date. Changes in this release are fully backward compatible.
All previous versions of this package will be deprecated. We recommend upgrading to version 4.0.0 as soon as possible before February 1st, 2025.
So no extra changes are necessary apparently.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you move this to separate PR? It will not make much logical sense to put it in same PR and the change is still failing. It seems that there are now using v4 as version.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I prefer to keep this change here to keep the process simple, specially because this doesn't change the feature being added.
|
Wow, this stuff is confusing. There seems to be different things both called cache, or the link is just confusing (it mentions actions/toolkit). There doesn't seem to be a v4.0.5 for actions/cache. I'd say just use |
Yes, the original error message was also misleading about the support versions too, since it said that v3 was also valid. |
|
I merged the changes, now you can create the bump version PR. |
Motivation:
dpiattribute as a parameterdpiafter callingcreate_schema_graphis cumbersome:graph.obj_dict["attributes"]["dpi"] = 300Proposed changes:
format_graphparameterImage creation examples:
Current implementation without dpi adjustment:
Proposed implementation: