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

Fix URL joining in OptimadeRester #3613

Merged
merged 3 commits into from
Feb 9, 2024
Merged

Fix URL joining in OptimadeRester #3613

merged 3 commits into from
Feb 9, 2024

Conversation

ricardonpa
Copy link
Contributor

Summary

Major changes:

  • fix 1:
    • Fix URL joining in OptimadeRester. The original os.path.join function is not the correct function to use for joining URLs. It's designed for joining file paths, and its behavior depends on the operating system. On Windows, it uses a backslash () as the separator, while on Unix-based systems, it uses a forward slash (/).
    • This is a small fix, so no tests were added to 'test_optimade.py'
    • This fix should resolve the issue reported at: https://matsci.org/t/could-not-retrieve-information-from-optimade/39469

Todos

None

Checklist

  • [N/A] Google format doc strings added. Check with ruff.
  • [N/A] Type annotations included. Check with mypy.
  • [N/A] Tests added for new features/fixes.
  • [N/A] If applicable, new classes/functions/modules have duecredit @due.dcite decorators to reference relevant papers by DOI (example)

Tip: Install pre-commit hooks to auto-check types and linting before every commit:

pip install -U pre-commit
pre-commit install

ricardonpa and others added 3 commits February 8, 2024 22:57
The original os.path.join function is not the correct function to use for joining URLs. It's designed for joining file paths, and its behavior depends on the operating system. On Windows, it uses a backslash (\) as the separator, while on Unix-based systems, it uses a forward slash (/).

Signed-off-by: Ricardo Amaral <rna5137@psu.edu>
Copy link
Member

@janosh janosh left a comment

Choose a reason for hiding this comment

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

thanks a lot @rdamaral for submitting a fix!

@janosh janosh enabled auto-merge (squash) February 9, 2024 14:53
@janosh janosh merged commit cf1b61a into materialsproject:master Feb 9, 2024
22 checks passed
@ricardonpa ricardonpa deleted the optimade-urljoin-fix branch February 13, 2024 22:02
@janosh janosh added fix Bug fix PRs api Application programming interface labels Feb 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api Application programming interface fix Bug fix PRs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants