Skip to content

remove breaking changes that load on init#406

Merged
blue442 merged 1 commit intomainfrom
fix_main
Nov 9, 2023
Merged

remove breaking changes that load on init#406
blue442 merged 1 commit intomainfrom
fix_main

Conversation

@ascourtas
Copy link
Copy Markdown
Contributor

Fix up main to revert load on init() changes previously merged, but keep flake8 fixes, comments, docs, metadata validation, and minor tweaks. This means we preserve previous functionality for users when loading datasets, but keep all other changes.

This should keep main "clean" such that we can cut a minor/patch release before the new code-breaking changes of the class refactor are merged.

@what-the-diff
Copy link
Copy Markdown

what-the-diff Bot commented Oct 30, 2023

PR Summary

  • Improved Functionality in foundry.py

    • Streamlined the initialization process by removing less essential parameters such as name.
    • Simplified data access by setting download and globus parameters to default to True.
    • Enhanced code simplicity by replacing the _load function with an easier to use load function which now supports keyword arguments.
    • Although the interval parameter was removed from the load function, this doesn't affect the overall functionality.
    • Included a proposed solution (commented out for now) for the publish_dataset function which may be implemented in future updates.
  • Enhanced Test Coverage in test_foundry.py:

    • Updated the test cases to align with the changes made in foundry.py and removed the test_dataset parameter from Foundry initialization.
    • More features of Foundry are now tested by including a call to load function after its initialization in various test functions.
    • Test reliability improved by adding a call to _delete_test_data function at the beginning of each test, ensuring a fresh test environment every time.
    • Removed the download, globus, no_browser, and no_local_server keyword arguments in the Foundry initialization in test functions as these are now set to default values for a more streamlined testing process.

@ascourtas ascourtas mentioned this pull request Oct 31, 2023
4 tasks
Copy link
Copy Markdown
Collaborator

@blue442 blue442 left a comment

Choose a reason for hiding this comment

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

Checked diffs for a couple different commits, and it looks like all of the target functionality was removed while leaving the rest - LGTM!

@blue442 blue442 merged commit 3f8a0fa into main Nov 9, 2023
@blue442 blue442 deleted the fix_main branch November 9, 2023 19:47
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