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

Do not enforce min/max on sequence index column #2043

Merged
merged 4 commits into from
Jun 5, 2024

Conversation

lajohn4747
Copy link
Contributor

resolves #2031
CU-86b0me61g

Do not enforce min/max on sequence index column so it does not cause duplication of the seq_index

@lajohn4747 lajohn4747 requested a review from a team as a code owner May 31, 2024 20:03
@sdv-team
Copy link
Contributor

@lajohn4747 lajohn4747 added this to the 1.13.2 milestone Jun 3, 2024
Comment on lines 220 to 221
if sequence_index_transformer.enforce_min_max_values:
sequence_index_transformer.enforce_min_max_values = False
Copy link
Contributor

Choose a reason for hiding this comment

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

If a user updated the sequence index transformer and set enforce_min_max_values to True, would this ignore that? It might be better to only change the transformer when it's been automatically assigned, so that way if a user modifies it we don't overwrite their change.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sounds good! I moved it into a overridden auto_assign_method so it's always after being auto assigned.

@lajohn4747 lajohn4747 requested a review from frances-h June 3, 2024 20:35
Copy link
Contributor

@frances-h frances-h left a comment

Choose a reason for hiding this comment

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

LGTM! 👍

@lajohn4747 lajohn4747 merged commit 70c2bf7 into main Jun 5, 2024
39 checks passed
@lajohn4747 lajohn4747 deleted the issue_2031_duplicate_s_index branch June 5, 2024 22:34
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.

PARSynthesizer: Duplicate sequence index values when sequence_length is higher than real data
4 participants