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

Support more stypes in LinearModelEncoder #325

Merged
merged 15 commits into from
Jan 9, 2024
Merged

Support more stypes in LinearModelEncoder #325

merged 15 commits into from
Jan 9, 2024

Conversation

zechengz
Copy link
Member

@zechengz zechengz commented Jan 3, 2024 •
edited
Loading

@zechengz zechengz self-assigned this Jan 3, 2024
@github-actions github-actions bot added the nn label Jan 3, 2024
@zechengz zechengz changed the title [1 / N] Support more stypes in LinearModelEncoder [1 / N] Support some more stypes in LinearModelEncoder Jan 3, 2024
@codecov Codecov
Copy link

codecov bot commented Jan 3, 2024 •
edited
Loading

Codecov Report

Attention: 1 lines in your changes are missing coverage. Please review.

Comparison is base (eb4f81b) 93.02% compared to head (38e45b9) 93.12%.

Files Patch % Lines
test/nn/encoder/test_stype_encoder.py 98.70% 1 Missing ⚠️
Additional details and impacted files 
@@            Coverage Diff             @@
##           master     #325      +/-   ##
==========================================
+ Coverage   93.02%   93.12%   +0.09%     
==========================================
  Files         117      117              
  Lines        6024     6107      +83     
==========================================
+ Hits         5604     5687      +83     
  Misses        420      420

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@zechengz zechengz marked this pull request as ready for review January 3, 2024 08:15
weihua916
weihua916 reviewed Jan 3, 2024
View reviewed changes
torch_frame/nn/encoder/stype_encoder.py Outdated Show resolved Hide resolved
torch_frame/nn/encoder/stype_encoder.py Outdated Show resolved Hide resolved
torch_frame/nn/encoder/stype_encoder.py Show resolved Hide resolved
test/nn/encoder/test_stype_encoder.py Show resolved Hide resolved
torch_frame/nn/encoder/stype_encoder.py Show resolved Hide resolved
test/nn/encoder/test_stype_encoder.py Outdated Show resolved Hide resolved
test/nn/encoder/test_stype_encoder.py Outdated Show resolved Hide resolved
test/nn/encoder/test_stype_encoder.py Outdated Show resolved Hide resolved
torch_frame/nn/encoder/stype_encoder.py Outdated Show resolved Hide resolved
@github-actions github-actions bot added the data label Jan 5, 2024
@zechengz zechengz changed the title [1 / N] Support some more stypes in LinearModelEncoder Support more stypes in LinearModelEncoder Jan 5, 2024
weihua916
weihua916 reviewed Jan 5, 2024
View reviewed changes
Copy link
Contributor

@weihua916 weihua916 left a comment

Choose a reason for hiding this comment

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

Mostly good. Major request is to drop max seq len stats and support for sequence numerical.

torch_frame/data/stats.py Outdated Show resolved Hide resolved
test/nn/encoder/test_stype_encoder.py Outdated Show resolved Hide resolved
test/nn/encoder/test_stype_encoder.py Outdated Show resolved Hide resolved
test/nn/encoder/test_stype_encoder.py Outdated Show resolved Hide resolved
test/nn/encoder/test_stype_encoder.py Outdated Show resolved Hide resolved
torch_frame/nn/encoder/stype_encoder.py Outdated Show resolved Hide resolved
torch_frame/nn/encoder/stype_encoder.py Outdated Show resolved Hide resolved
@github-actions github-actions bot removed the data label Jan 8, 2024
weihua916
weihua916 approved these changes Jan 8, 2024
View reviewed changes
Copy link
Contributor

@weihua916 weihua916 left a comment

Choose a reason for hiding this comment

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

Looks great. thank you.

torch_frame/nn/encoder/stype_encoder.py Outdated Show resolved Hide resolved
torch_frame/nn/encoder/stype_encoder.py Outdated Show resolved Hide resolved
@zechengz zechengz merged commit 46e20d5 into master Jan 9, 2024
13 checks passed
@zechengz zechengz deleted the zecheng_add_linear_model_encoder_stype branch January 9, 2024 05:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@weihua916 weihua916 weihua916 approved these changes

Assignees

@zechengz zechengz

Labels
nn
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Support more stype in LinearModelEncoder
2 participants
@zechengz @weihua916