-
Notifications
You must be signed in to change notification settings - Fork 326
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
Wrong Assumption in ScalarRamge Condition Considering Integer as Datetime #2311
Comments
Hi @Deathn0t, it’s great to see your interest in the SDV ecosystem. The SDV software (and its related libraries) is owned and maintained by DataCebo. It is available under the Business Source License for you to browse. We are a small team supporting a large set of users with enterprise-specific intricacies and reliability needs. This has required us to be deliberate about setting the roadmap for SDV libraries. As a result, we are unable to prioritize reviewing and accepting external pull requests. Is your PR is meant to address a bug or feature request? We would greatly appreciate it if you could file an issue instead with the overall description of your problem. We can determine whether it’s aligned with our framework. Once discussed, our team typically resolves smaller issues within a few release cycles. We appreciate your understanding. |
I suggest updating the condition that checks for The current condition resulted in an exception when the range values could be interpreted as dates (e.g., 1900, 2000) even though the column type is So, I suggested that you raise the exception only when the column is a |
Thanks for describing this @Deathn0t. I have filed a bug for it in #2324 so that we can track the fix. Please check it out and let us know if the error you are describing is different than what's in the bug. In this issue, I have also provided a simple workaround that can unblock your project for now (casting the column to a string). At the bottom of this issue, I have also provided some additional context that may be useful (copied below)
Regarding the second point: coincidentally the team is planning to streamline constraints in Q1, so we might actually resolve this bug as part of this project. |
If I understand #2324 correctly. Then what I describe is different. I have a column, for example named I tracked the line that does this already: https://github.com/sdv-dev/SDV/pull/2150/files#diff-4dc23c81a73c7caca3948f0875ffef02ec33cad02f24834c282eac20b1d13586R1139 |
Hi @Deathn0t, does this other issue #2328 capture what you are observing? It's helpful for me to understand how to replicate this problem. If this is something that is actively blocking your projects, I have left a workaround in #2328 that you can use in the meantime. Re code: I am not as familiar with the internals of this part of our code. However, considering both issues together, it seems like we are generally making some assumptions about data types that shouldn't be needed. Refactoring our entire logic for data validation here may make sense. I think the only validation we really need is:
As I mentioned before, the team will soon do a refactor of all constraints in Q1, so this may be something we save for then (should be soon). BTW: On a separate note, I am curious about why you'd like to apply the |
Hi @npatki , yes #2328 is exactly corresponding to my issue! That's "great" you managed to reproduce it as a few updates have come up since I opened the PR. I am using SDV within Hyperparameter optimization (HPO). To transfer the learning from the previous HPO to the new HPO. It is really just fitting an SDV sampler on previous filtered best outcomes from HPO and then using this sampler within a Bayesian optimization code. For this software https://github.com/deephyper/deephyper, described in this paper. |
Hi @Deathn0t! It’s great to see your interest in the SDV ecosystem. This comment is a reminder to consult your legal team before adopting the SDV into your project, as SDV has a source-available license. For more information, you can read through our license FAQs and blog article (not legal advice). For any other questions, you can Contact Us. You can also inquire about a commercial license to allow additional use. |
Hi @Deathn0t just wanted to follow up on this, as this issue has been inactive for some time. I think we can close it off in favor of #2328. Though I think that both #2328 and #2324 are due to the same underlying root cause of making inaccurate assumptions about when something is supposed to be a datetime. Do feel free to reply down below if you have more to discuss, as I can always re-open the issue. Thanks. |
Hello, I submitted a PR in July for a small code change related to the automated detection of datetime (that happened when it should not).
#2150
The PR has not received any feedback/comment. Will this be ignored?
Thank you very much for your help.
The text was updated successfully, but these errors were encountered: