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.

Sign up for GitHub

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

Jump to bottom

[FSTORE-1328] Add list to feature store and richer repr drafts #1301

Open
wants to merge 27 commits into
base: master
Choose a base branch
from
Open

[FSTORE-1328] Add list to feature store and richer repr drafts #1301

wants to merge 27 commits into from

Conversation

vatj
Copy link
Contributor

@vatj vatj commented Apr 23, 2024

This PR adds/fixes/changes...

  • please summarize your changes to the code
  • and make sure to include all changes to user-facing APIs

JIRA Issue: -

Priority for Review: -

Related PRs: -

How Has This Been Tested?

  • Unit Tests
  • Integration Tests
  • Manual Tests on VM

Checklist For The Assigned Reviewer:

- [ ] Checked if merge conflicts with master exist
- [ ] Checked if stylechecks for Java and Python pass
- [ ] Checked if all docstrings were added and/or updated appropriately
- [ ] Ran spellcheck on docstring
- [ ] Checked if guides & concepts need to be updated
- [ ] Checked if naming conventions for parameters and variables were followed
- [ ] Checked if private methods are properly declared and used
- [ ] Checked if hard-to-understand areas of code are commented
- [ ] Checked if tests are effective
- [ ] Built and deployed changes on dev VM and tested manually
- [x] (Checked if all type annotations were added and/or updated appropriately)

vatj added 22 commits April 21, 2024 16:26
@vatj
Scaffold some structure for python SDK improved communication
faa1add
@vatj
Add welcome message on connection
182ba48
@vatj
Minor adjustments
b443792
@vatj
Set styles like if I had a taste for these things
4533b5c
@vatj
Add api for get_all feature groups
c581a5a
@vatj
Add base list and table printers
62f1a1b
@vatj
Add api call for get_all feature views
d7e617c
@vatj
Add feature_store quicktour
5616255
@vatj
Minor fix and clean ups
a7d59e6
@vatj
Add user_messages file
651be27
@vatj
Update base tables for FGs
074562d
@vatj
Add methods to feature view engine
9845179
@vatj
Add alternate table style for fg list
b0b616a
@vatj
More fixes
8e21bc1
@vatj
Filter by latest version for feature view api
57ddf87
@vatj
Refactor + adapt feature view table
868fbbb
@vatj
Refactoring
06e7298
@vatj
Clean up
bca6b74
@vatj
Basic FV info draft
d77b784
@vatj
More table info for feature views
521695b
@vatj
More table info for feature views
e7184c9
@vatj
Minor adjustments
d912ef5
@vatj vatj requested review from dhananjay-mk and javierdlrm and removed request for dhananjay-mk April 23, 2024 07:53
dhananjay-mk
dhananjay-mk reviewed Apr 25, 2024
View reviewed changes
Copy link
Contributor

@dhananjay-mk dhananjay-mk left a comment

Choose a reason for hiding this comment

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

minor comments on quick look

python/hsfs/core/feature_group_engine.py
fgroup
for fgroup in fg_list
if fgroup.version
== max(fg1.version for fg1 in fg_list if fg1.name == fgroup.name)
Copy link
Contributor

Choose a reason for hiding this comment

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

may be using filter() is better for readability instead of having a for loop again

python/hsfs/helpers/quicktours/feature_store.md

## Get or Create Storage Connectors to access External Data

You can retrieve or create `StorageConnector` objects using the `get_or_create_storage_connector()` method. `StorageConnector`s objects are used to access external data sources, such as S3, Snowflake, BigQuery. Each `StorageConnector` has its own properties so check out the docs for the specific data source you are interested in:
Copy link
Contributor

Choose a reason for hiding this comment

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

we dont have create storage connector method, there is only get_storage_connector

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dhananjay-mk dhananjay-mk dhananjay-mk left review comments

@javierdlrm javierdlrm Awaiting requested review from javierdlrm

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@vatj @dhananjay-mk