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

validate_paths data prefix #728

Merged
merged 4 commits into from
Sep 27, 2022
Merged

validate_paths data prefix #728

merged 4 commits into from
Sep 27, 2022

Conversation

camisowers
Copy link
Contributor

@camisowers camisowers commented Sep 27, 2022

If you haven't already, please read through our contributing guidelines before opening your PR

What is the purpose of this PR?

Closes #676.

How did you implement your changes

Check for data prefix first if specified and change default to data_prefix=False. The instances of validate_paths in the notebook use the default which now doesn't require ..\data.

Remaining issues

After this PR is merged I'll check throughout toffy for appropriate behavior using the False default.

@camisowers camisowers added the bug Something isn't working label Sep 27, 2022
@camisowers camisowers self-assigned this Sep 27, 2022
@camisowers camisowers marked this pull request as ready for review September 27, 2022 17:29
Copy link
Member

@ngreenwald ngreenwald left a comment

Choose a reason for hiding this comment

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

Do any of the notebooks use this function with data_prefix set to True? Or other functions in ark?

@camisowers
Copy link
Contributor Author

Nope! The notebooks just use the default and all the ark functions have data_prefix=False.

@ngreenwald ngreenwald merged commit d36223f into main Sep 27, 2022
@ngreenwald ngreenwald deleted the data_prefix branch September 27, 2022 18:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Appropriate behavior for data_prefix=True in validate_paths
2 participants