-
Notifications
You must be signed in to change notification settings - Fork 790
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
Stacked @card v1.2 : Test cases for Multiple @card
s
#898
Merged
valayDave
merged 105 commits into
mfcards-s4-ns-packages
from
mfcards-s5-multi-card-testcases
Jan 24, 2022
Merged
Stacked @card v1.2 : Test cases for Multiple @card
s
#898
valayDave
merged 105 commits into
mfcards-s4-ns-packages
from
mfcards-s5-multi-card-testcases
Jan 24, 2022
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- added argument for tests
…nents' into multiple-card-decorators-test-cases
- Added `list_cards` to `CliCheck` and `MetadataCheck` - Bringing #875 into the code - Added a card that prints taskspec with random number
- `current.cards['myid']` should be accessible when cards have an `id` argument in decorator - `current.cards.append` should not work when there are no single default editable card. - if a card has `ALLOW_USER_COMPONENTS=False` then it can still be edited via accessing it with `id` property. - adding arbitrary information to `current.cards.append` should not break user code. - Only cards with `ALLOW_USER_COMPONENTS=True` are considered default editable. - If a single @card decorator is present with `id` then it `current.cards.append` should still work - Access of `current.cards` with non existant id should not fail. - `current.cards.append` should be accessible to the card with `customize=True`. -
- Fixed comment - Fixed the `customize=True` test case.
…nents' into multiple-card-decorators-test-cases
…nents' into multiple-card-decorators-test-cases
…nents' into multiple-card-decorators-test-cases
…nents' into multiple-card-decorators-test-cases
…nents' into multiple-card-decorators-test-cases
…nents' into multiple-card-decorators-test-cases
…nents' into multiple-card-decorators-test-cases
- ensure entropy of rendered information is high enough to not overwrite a file.
…nents' into multiple-card-decorators-test-cases
…-custom-components-final-api-def
…f' into multiple-card-decorators-read-cli
…ecorators-custom-components-actual-components
…nents' into multiple-card-decorators-test-cases
…nents' into multiple-card-decorators-test-cases
…nents' into multiple-card-decorators-test-cases
…nents' into multiple-card-decorators-test-cases
…ple-card-decorators-test-cases
…ple-card-decorators-test-cases
…ple-card-decorators-test-cases
…ple-card-decorators-test-cases
…f' into multiple-card-decorators-test-cases
…ard-decorators-namespace-packages
…ple-card-decorators-test-cases
…nents' into multiple-card-decorators-graph-changes
…ard-decorators-namespace-packages
…f' into multiple-card-decorators-test-cases
…nents' into multiple-card-decorators-graph-changes
…ard-decorators-namespace-packages
…ple-card-decorators-test-cases
…lti-card-testcases
…lti-card-testcases
valayDave
added a commit
that referenced
this pull request
Jan 24, 2022
* setup import of cards from `metaflow_extensions` using `metaflow.extension_support` - Added import modules in `card_modules` - Added hook in card decorators to add custom packages * Added some debug statements for external imports. * Stacked @card v1.2 : Test cases for Multiple `@card`s (#898) * Multiple Cards Test Suite Mods (#27) - Added `list_cards` to `CliCheck` and `MetadataCheck` - Bringing #875 into the code - Added a card that prints taskspec with random number * Added test case for multiple cards * Added tests card, summary : - `current.cards['myid']` should be accessible when cards have an `id` argument in decorator - `current.cards.append` should not work when there are no single default editable card. - if a card has `ALLOW_USER_COMPONENTS=False` then it can still be edited via accessing it with `id` property. - adding arbitrary information to `current.cards.append` should not break user code. - Only cards with `ALLOW_USER_COMPONENTS=True` are considered default editable. - If a single @card decorator is present with `id` then it `current.cards.append` should still work - Access of `current.cards` with non existant id should not fail. - `current.cards.append` should be accessible to the card with `customize=True`. - * fixed `DefaultEditableCardTest` test case - Fixed comment - Fixed the `customize=True` test case. * ensure `test_pathspec_card` has no duplicates - ensure entropy of rendered information is high enough to not overwrite a file. * test case fix : `current.cards` to `current.card` * Added Test case to support import of cards. - Test case validates that broken card modules don't break metaflow - test case validates that we are able to import cards from metaflow_extensions - Test case validate that cards can be editable if they are importable. * Added Env var to tests to avoid warnings added to cards. * Added Test for card resume. * Stacked @card v1.2: Card Dev Docs (#899) Co-authored-by: Romain Cledat <[email protected]>
valayDave
added a commit
that referenced
this pull request
Jan 25, 2022
* accomodating changes from #833 - Minor tweaks to `graph.py` * Stacked @card v1.2 : Namespace Packages with `@card` (#897) * setup import of cards from `metaflow_extensions` using `metaflow.extension_support` - Added import modules in `card_modules` - Added hook in card decorators to add custom packages * Added some debug statements for external imports. * Stacked @card v1.2 : Test cases for Multiple `@card`s (#898) * Multiple Cards Test Suite Mods (#27) - Added `list_cards` to `CliCheck` and `MetadataCheck` - Bringing #875 into the code - Added a card that prints taskspec with random number * Added test case for multiple cards * Added tests card, summary : - `current.cards['myid']` should be accessible when cards have an `id` argument in decorator - `current.cards.append` should not work when there are no single default editable card. - if a card has `ALLOW_USER_COMPONENTS=False` then it can still be edited via accessing it with `id` property. - adding arbitrary information to `current.cards.append` should not break user code. - Only cards with `ALLOW_USER_COMPONENTS=True` are considered default editable. - If a single @card decorator is present with `id` then it `current.cards.append` should still work - Access of `current.cards` with non existant id should not fail. - `current.cards.append` should be accessible to the card with `customize=True`. - * fixed `DefaultEditableCardTest` test case - Fixed comment - Fixed the `customize=True` test case. * ensure `test_pathspec_card` has no duplicates - ensure entropy of rendered information is high enough to not overwrite a file. * test case fix : `current.cards` to `current.card` * Added Test case to support import of cards. - Test case validates that broken card modules don't break metaflow - test case validates that we are able to import cards from metaflow_extensions - Test case validate that cards can be editable if they are importable. * Added Env var to tests to avoid warnings added to cards. * Added Test for card resume. * Stacked @card v1.2: Card Dev Docs (#899) Co-authored-by: Romain Cledat <[email protected]>
valayDave
added a commit
that referenced
this pull request
Jan 25, 2022
* user-facing `MetaflowCardComponent`s - import via `from metaflow.cards import Artifact,..` * 7 Major Changes: - Removed `Section` component from `metaflow.cards` - Making user-facing component inherent to `UserComponent` which in turn inherits `MetaflowCardComponent` - Wrap all `UserComponents` inside a section after rendering everything per card - created a `render_safely` decorator to ensure fail-safe render of `UserComponent`s - removed code from component serializer which used internal components - Refactored some components that return render - Added docstrings to all components. * JS + CSS + Cards UI Build * Stacked @card v1.2 : Graph Related Changes to card cli (#911) * accomodating changes from #833 - Minor tweaks to `graph.py` * Stacked @card v1.2 : Namespace Packages with `@card` (#897) * setup import of cards from `metaflow_extensions` using `metaflow.extension_support` - Added import modules in `card_modules` - Added hook in card decorators to add custom packages * Added some debug statements for external imports. * Stacked @card v1.2 : Test cases for Multiple `@card`s (#898) * Multiple Cards Test Suite Mods (#27) - Added `list_cards` to `CliCheck` and `MetadataCheck` - Bringing #875 into the code - Added a card that prints taskspec with random number * Added test case for multiple cards * Added tests card, summary : - `current.cards['myid']` should be accessible when cards have an `id` argument in decorator - `current.cards.append` should not work when there are no single default editable card. - if a card has `ALLOW_USER_COMPONENTS=False` then it can still be edited via accessing it with `id` property. - adding arbitrary information to `current.cards.append` should not break user code. - Only cards with `ALLOW_USER_COMPONENTS=True` are considered default editable. - If a single @card decorator is present with `id` then it `current.cards.append` should still work - Access of `current.cards` with non existant id should not fail. - `current.cards.append` should be accessible to the card with `customize=True`. - * fixed `DefaultEditableCardTest` test case - Fixed comment - Fixed the `customize=True` test case. * ensure `test_pathspec_card` has no duplicates - ensure entropy of rendered information is high enough to not overwrite a file. * test case fix : `current.cards` to `current.card` * Added Test case to support import of cards. - Test case validates that broken card modules don't break metaflow - test case validates that we are able to import cards from metaflow_extensions - Test case validate that cards can be editable if they are importable. * Added Env var to tests to avoid warnings added to cards. * Added Test for card resume. * Stacked @card v1.2: Card Dev Docs (#899) Co-authored-by: Romain Cledat <[email protected]> Co-authored-by: Brendan Gibson <[email protected]> Co-authored-by: Brendan Gibson <[email protected]> Co-authored-by: adam <[email protected]> Co-authored-by: Romain Cledat <[email protected]>
valayDave
added a commit
that referenced
this pull request
Jan 25, 2022
…895) * Added the `--id` option to `card view`/`card get` - modified datastore and fixed the exception class. - Allowing some ambiguity in pathspec argument for `get/view` command. * Added `id` argument to `get_cards` * Bringing `card list` cli functionality from test-suite branch - added functionality to list cards about a task and even list it as JSON. * changed hash checking logic. - instead of equating we check `startswith` * Added list many feature for `card list` - listing all cards from the latest run when card list is called with no argument * Added `card list` print formatting changes - `--as-json` works for many cards and single cards * Stacked @card v1.2 : New `MetaflowCardComponent`s (#896) * user-facing `MetaflowCardComponent`s - import via `from metaflow.cards import Artifact,..` * 7 Major Changes: - Removed `Section` component from `metaflow.cards` - Making user-facing component inherent to `UserComponent` which in turn inherits `MetaflowCardComponent` - Wrap all `UserComponents` inside a section after rendering everything per card - created a `render_safely` decorator to ensure fail-safe render of `UserComponent`s - removed code from component serializer which used internal components - Refactored some components that return render - Added docstrings to all components. * JS + CSS + Cards UI Build * Stacked @card v1.2 : Graph Related Changes to card cli (#911) * accomodating changes from #833 - Minor tweaks to `graph.py` * Stacked @card v1.2 : Namespace Packages with `@card` (#897) * setup import of cards from `metaflow_extensions` using `metaflow.extension_support` - Added import modules in `card_modules` - Added hook in card decorators to add custom packages * Added some debug statements for external imports. * Stacked @card v1.2 : Test cases for Multiple `@card`s (#898) * Multiple Cards Test Suite Mods (#27) - Added `list_cards` to `CliCheck` and `MetadataCheck` - Bringing #875 into the code - Added a card that prints taskspec with random number * Added test case for multiple cards * Added tests card, summary : - `current.cards['myid']` should be accessible when cards have an `id` argument in decorator - `current.cards.append` should not work when there are no single default editable card. - if a card has `ALLOW_USER_COMPONENTS=False` then it can still be edited via accessing it with `id` property. - adding arbitrary information to `current.cards.append` should not break user code. - Only cards with `ALLOW_USER_COMPONENTS=True` are considered default editable. - If a single @card decorator is present with `id` then it `current.cards.append` should still work - Access of `current.cards` with non existant id should not fail. - `current.cards.append` should be accessible to the card with `customize=True`. - * fixed `DefaultEditableCardTest` test case - Fixed comment - Fixed the `customize=True` test case. * ensure `test_pathspec_card` has no duplicates - ensure entropy of rendered information is high enough to not overwrite a file. * test case fix : `current.cards` to `current.card` * Added Test case to support import of cards. - Test case validates that broken card modules don't break metaflow - test case validates that we are able to import cards from metaflow_extensions - Test case validate that cards can be editable if they are importable. * Added Env var to tests to avoid warnings added to cards. * Added Test for card resume. * Stacked @card v1.2: Card Dev Docs (#899) Co-authored-by: Romain Cledat <[email protected]> Co-authored-by: Brendan Gibson <[email protected]> Co-authored-by: Brendan Gibson <[email protected]> Co-authored-by: adam <[email protected]> Co-authored-by: Romain Cledat <[email protected]> Co-authored-by: Brendan Gibson <[email protected]> Co-authored-by: Brendan Gibson <[email protected]> Co-authored-by: adam <[email protected]> Co-authored-by: Romain Cledat <[email protected]>
valayDave
added a commit
that referenced
this pull request
Jan 25, 2022
* allow passing userland `MetaflowCardComponents` - Added api for `current.cards` that handles multiple decorators * `current.cards` with `id` support. - Introduced `ALLOW_USER_COMPONENTS` attribute to `MetaflowCard` class - setting `ALLOW_USER_COMPONENTS=True` allow editable cards - Modified logic of `CardComponentCollector` - `CardComponentCollector.append` only accessible to default editble card * Added `customize=True` in `@card` - `customize=True` only allowed for one @card deco per @step - `customize=True` sets the card to be default editable - `current.card.append` appends to @card with `customize=True` * Stacked @card v1.2: Read cli changes for Supporting Multiple`@card`s (#895) * Added the `--id` option to `card view`/`card get` - modified datastore and fixed the exception class. - Allowing some ambiguity in pathspec argument for `get/view` command. * Added `id` argument to `get_cards` * Bringing `card list` cli functionality from test-suite branch - added functionality to list cards about a task and even list it as JSON. * changed hash checking logic. - instead of equating we check `startswith` * Added list many feature for `card list` - listing all cards from the latest run when card list is called with no argument * Added `card list` print formatting changes - `--as-json` works for many cards and single cards * Stacked @card v1.2 : New `MetaflowCardComponent`s (#896) * user-facing `MetaflowCardComponent`s - import via `from metaflow.cards import Artifact,..` * 7 Major Changes: - Removed `Section` component from `metaflow.cards` - Making user-facing component inherent to `UserComponent` which in turn inherits `MetaflowCardComponent` - Wrap all `UserComponents` inside a section after rendering everything per card - created a `render_safely` decorator to ensure fail-safe render of `UserComponent`s - removed code from component serializer which used internal components - Refactored some components that return render - Added docstrings to all components. * JS + CSS + Cards UI Build * Stacked @card v1.2 : Graph Related Changes to card cli (#911) * accomodating changes from #833 - Minor tweaks to `graph.py` * Stacked @card v1.2 : Namespace Packages with `@card` (#897) * setup import of cards from `metaflow_extensions` using `metaflow.extension_support` - Added import modules in `card_modules` - Added hook in card decorators to add custom packages * Added some debug statements for external imports. * Stacked @card v1.2 : Test cases for Multiple `@card`s (#898) * Multiple Cards Test Suite Mods (#27) - Added `list_cards` to `CliCheck` and `MetadataCheck` - Bringing #875 into the code - Added a card that prints taskspec with random number * Added test case for multiple cards * Added tests card, summary : - `current.cards['myid']` should be accessible when cards have an `id` argument in decorator - `current.cards.append` should not work when there are no single default editable card. - if a card has `ALLOW_USER_COMPONENTS=False` then it can still be edited via accessing it with `id` property. - adding arbitrary information to `current.cards.append` should not break user code. - Only cards with `ALLOW_USER_COMPONENTS=True` are considered default editable. - If a single @card decorator is present with `id` then it `current.cards.append` should still work - Access of `current.cards` with non existant id should not fail. - `current.cards.append` should be accessible to the card with `customize=True`. - * fixed `DefaultEditableCardTest` test case - Fixed comment - Fixed the `customize=True` test case. * ensure `test_pathspec_card` has no duplicates - ensure entropy of rendered information is high enough to not overwrite a file. * test case fix : `current.cards` to `current.card` * Added Test case to support import of cards. - Test case validates that broken card modules don't break metaflow - test case validates that we are able to import cards from metaflow_extensions - Test case validate that cards can be editable if they are importable. * Added Env var to tests to avoid warnings added to cards. * Added Test for card resume. * Stacked @card v1.2: Card Dev Docs (#899) Co-authored-by: Brendan Gibson <[email protected]> Co-authored-by: Brendan Gibson <[email protected]> Co-authored-by: adam <[email protected]> Co-authored-by: Romain Cledat <[email protected]>
oavdeev
pushed a commit
that referenced
this pull request
Jan 25, 2022
* multiple decorator support * Fixing comments * comment fix. * commet fix * allow multiple decorators of same type from cli * Stacked @card v1.2 : Customizing `@card` with `current.card` (#894) * allow passing userland `MetaflowCardComponents` - Added api for `current.cards` that handles multiple decorators * `current.cards` with `id` support. - Introduced `ALLOW_USER_COMPONENTS` attribute to `MetaflowCard` class - setting `ALLOW_USER_COMPONENTS=True` allow editable cards - Modified logic of `CardComponentCollector` - `CardComponentCollector.append` only accessible to default editble card * Added `customize=True` in `@card` - `customize=True` only allowed for one @card deco per @step - `customize=True` sets the card to be default editable - `current.card.append` appends to @card with `customize=True` * Stacked @card v1.2: Read cli changes for Supporting Multiple`@card`s (#895) * Added the `--id` option to `card view`/`card get` - modified datastore and fixed the exception class. - Allowing some ambiguity in pathspec argument for `get/view` command. * Added `id` argument to `get_cards` * Bringing `card list` cli functionality from test-suite branch - added functionality to list cards about a task and even list it as JSON. * changed hash checking logic. - instead of equating we check `startswith` * Added list many feature for `card list` - listing all cards from the latest run when card list is called with no argument * Added `card list` print formatting changes - `--as-json` works for many cards and single cards * Stacked @card v1.2 : New `MetaflowCardComponent`s (#896) * user-facing `MetaflowCardComponent`s - import via `from metaflow.cards import Artifact,..` * 7 Major Changes: - Removed `Section` component from `metaflow.cards` - Making user-facing component inherent to `UserComponent` which in turn inherits `MetaflowCardComponent` - Wrap all `UserComponents` inside a section after rendering everything per card - created a `render_safely` decorator to ensure fail-safe render of `UserComponent`s - removed code from component serializer which used internal components - Refactored some components that return render - Added docstrings to all components. * JS + CSS + Cards UI Build * Stacked @card v1.2 : Graph Related Changes to card cli (#911) * accomodating changes from #833 - Minor tweaks to `graph.py` * Stacked @card v1.2 : Namespace Packages with `@card` (#897) * setup import of cards from `metaflow_extensions` using `metaflow.extension_support` - Added import modules in `card_modules` - Added hook in card decorators to add custom packages * Added some debug statements for external imports. * Stacked @card v1.2 : Test cases for Multiple `@card`s (#898) * Multiple Cards Test Suite Mods (#27) - Added `list_cards` to `CliCheck` and `MetadataCheck` - Bringing #875 into the code - Added a card that prints taskspec with random number * Added test case for multiple cards * Added tests card, summary : - `current.cards['myid']` should be accessible when cards have an `id` argument in decorator - `current.cards.append` should not work when there are no single default editable card. - if a card has `ALLOW_USER_COMPONENTS=False` then it can still be edited via accessing it with `id` property. - adding arbitrary information to `current.cards.append` should not break user code. - Only cards with `ALLOW_USER_COMPONENTS=True` are considered default editable. - If a single @card decorator is present with `id` then it `current.cards.append` should still work - Access of `current.cards` with non existant id should not fail. - `current.cards.append` should be accessible to the card with `customize=True`. - * fixed `DefaultEditableCardTest` test case - Fixed comment - Fixed the `customize=True` test case. * ensure `test_pathspec_card` has no duplicates - ensure entropy of rendered information is high enough to not overwrite a file. * test case fix : `current.cards` to `current.card` * Added Test case to support import of cards. - Test case validates that broken card modules don't break metaflow - test case validates that we are able to import cards from metaflow_extensions - Test case validate that cards can be editable if they are importable. * Added Env var to tests to avoid warnings added to cards. * Added Test for card resume. * Stacked @card v1.2: Card Dev Docs (#899) Co-authored-by: Brendan Gibson <[email protected]> Co-authored-by: Brendan Gibson <[email protected]> Co-authored-by: adam <[email protected]> Co-authored-by: Romain Cledat <[email protected]> * Nit fix in error. * Fixing bug in decorator * whitespace. * Changing import scheme of warning variable. * fixing warning suppression env var setting Co-authored-by: Brendan Gibson <[email protected]> Co-authored-by: Brendan Gibson <[email protected]> Co-authored-by: adam <[email protected]> Co-authored-by: Romain Cledat <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Stacked on #897
Test Suite Changes
The
MetadataCheck
and theCliCheck
objects containassert_card
andget_card
functions. This PR leverages thecard list
command introduced in #895 .card list
cli command is used bylist_cards
function in theCliCheck
. A similar function is added toMetadataCheck
. Both these functions returndict
s with the same structure.The
assert_card
function now also takescard_id
andcard_hash
as arguments. This makes filtering a single card easily possible.Test Cases
The test cases are derived from the changes we introduce in #894 . #894 introduces using components from userland i.e.
current.card
. The below test cases also test and validate the functioning of this user interface.current.card.append
/current.card.extend
Should work for:@card(type='x')
@card(type='x',id='a')
current.card
should not fail :current.card.append
shouldn't fail under ambiguity of resolutioncurrent.card['myid']
shouldn't fail ifmyid
doesn't existcurrent.card.get(type='nonexistingtype')
shouldn't fail.current.card.append(SomeRandomType())
shouldn't fail.current.card['myid']
should work for:current.card
should not fail on serialization when passing to subprocess. We should have graceful serialization failures which don't intrude in user space.Tests for
view
/get
input argument variations.multiple cards should work
@card
.