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

Simplify paths via pathlib #33

Merged

Conversation

kamarianakis
Copy link
Collaborator

@kamarianakis kamarianakis commented Sep 15, 2023

Retouch most paths to use pathlib (less errors in various OS)
Removed redundant shader files

Copy link
Collaborator

@aprotopsaltis aprotopsaltis Sep 21, 2023

Choose a reason for hiding this comment

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

fine, but for consistency with the rest of the files use double quotes

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

will do

Copy link
Collaborator

Choose a reason for hiding this comment

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

  • double quotes here too
  • I guess str() maybe removed just like the rest of the files

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

os.path.join return a str by default, whereas the pathlib returns a possix filepath, which is not the same and therefore both pyassimp and usd load functions do not automatically make the change possix-->str, yielding an error. I will add a comment on the code to make it clear, thanks

Copy link
Collaborator

Choose a reason for hiding this comment

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

double quotes

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

done

Copy link
Collaborator

@aprotopsaltis aprotopsaltis Sep 21, 2023

Choose a reason for hiding this comment

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

  • is str() needed in load ?
  • if not then remove and use " instead of single quotes

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

fixed, see also comment above

Copy link
Collaborator

@aprotopsaltis aprotopsaltis Sep 21, 2023

Choose a reason for hiding this comment

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

  • is str() really needed in pyassimp.load ?
  • for consistency, use " instead of single quotes

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

fixed, added comment for str, moved it to separate line for clarity

Copy link
Collaborator

@aprotopsaltis aprotopsaltis Sep 21, 2023

Choose a reason for hiding this comment

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

for consistency with the rest, create a variable atlasPath, using double quotes

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

done

@kamarianakis kamarianakis merged commit f9c974a into papagiannakis:develop Sep 21, 2023
@kamarianakis kamarianakis deleted the feature/change_path_library branch September 21, 2023 11:48
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