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

style: use ruff instead of prospector #28

Merged
merged 12 commits into from
Mar 1, 2024
Merged

style: use ruff instead of prospector #28

merged 12 commits into from
Mar 1, 2024

Conversation

DaniBodor
Copy link
Member

No description provided.

@wbaccinelli
Copy link
Collaborator

wbaccinelli commented Feb 23, 2024

To be checked the PathLib issue

@DaniBodor
Copy link
Member Author

To be checked the PathLib issie

Could you please test it again on the last commit, and if that doesn't work on the commit before (1dbf35f).
If neither of those work, then I will just move back to using os instead of pathlib.

Semi-related to this: is the get_parent_directory function doing anything at this point? It is not referenced in the rest of the code, and it seems I can remove it and the dashboard still works.

@wbaccinelli
Copy link
Collaborator

To be checked the PathLib issie

Could you please test it again on the last commit, and if that doesn't work on the commit before (1dbf35f). If neither of those work, then I will just move back to using os instead of pathlib.

I tried on both the commits, but I get the same error. The problem was that the with Path the output was not a string but a path object an this is not accepted by the some of the dash core components. I am pushing the changes.

Semi-related to this: is the get_parent_directory function doing anything at this point? It is not referenced in the rest of the code, and it seems I can remove it and the dashboard still works.

Yes, it is used when you click "parent directory" in the popup. Unfortunately, with Dash it's not easy to retrieve immediately if a callback function is actually used, because it is not directly referenced in the code, but it's triggered when the input in the decorator is changed.

@DaniBodor DaniBodor force-pushed the ruff_dbodor branch 2 times, most recently from b5d2648 to e51f1a7 Compare February 29, 2024 13:34
@DaniBodor
Copy link
Member Author

I tried on both the commits, but I get the same error. The problem was that the with Path the output was not a string but a path object an this is not accepted by the some of the dash core components. I am pushing the changes.

Thanks! I squashed the three commits regarding pathlib into a single commit as they together fixed a single thing.

@DaniBodor
Copy link
Member Author

@wbaccinelli , I still need you to approve the PR before I can merge :)

Copy link
Collaborator

@wbaccinelli wbaccinelli left a comment

Choose a reason for hiding this comment

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

Tested and it works :)

Comment on lines 13 to 19
"notebook.lineNumbers": "on",
"notebook.formatOnSave.enabled": true,
"notebook.codeActionsOnSave": {
"notebook.source.fixAll": "explicit"
},
"notebook.diff.ignoreMetadata": true,
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think we can get rid of this

Copy link
Member Author

@DaniBodor DaniBodor Mar 1, 2024

Choose a reason for hiding this comment

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

agreed. I removed it from that commit.

@DaniBodor DaniBodor force-pushed the ruff_dbodor branch 2 times, most recently from 0f62ce7 to 0ad4b43 Compare March 1, 2024 09:10
@DaniBodor DaniBodor merged commit e8cabeb into main Mar 1, 2024
1 of 2 checks passed
@DaniBodor DaniBodor deleted the ruff_dbodor branch March 1, 2024 09:11
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.

2 participants