Skip to content

Make DataSignal construction more flexible #1303

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

Merged
merged 1 commit into from
Oct 11, 2023

Conversation

melange396
Copy link
Collaborator

Pass only reflexively valid fields to the DataSignal constructor when loading from db_signals.csv, instead of using a rigid and brittle set of fields (columns) to exclude. The system will now easily ignore new columns added to the CSV file, but for which it doesn't yet have a use. This will make it effortlessly handle changes coming in #1259 (and beyond).

I considered logging a list of unused fields/columns, but that will already be noisy because of the existing base_is_other column, and i dont think itll be very helpful.

@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@melange396 melange396 requested a review from dmytrotsko October 10, 2023 22:08
Copy link
Contributor

@dmytrotsko dmytrotsko left a comment

Choose a reason for hiding this comment

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

👍

@melange396 melange396 merged commit 9c3e8a3 into dev Oct 11, 2023
@melange396 melange396 deleted the flexible_datasignal_construction branch October 11, 2023 15:43
melange396 added a commit that referenced this pull request Jul 25, 2024
...as was done for DataSignal() in #1303 , and using the same ugly over-extended comprehension pattern
melange396 added a commit that referenced this pull request Jul 29, 2024
* chore: update docs

* restrict DataSource() construction to only applicable fields
...as was done for DataSignal() in #1303 , and using the same ugly over-extended comprehension pattern

---------

Co-authored-by: melange396 <[email protected]>
Co-authored-by: george haff <[email protected]>
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