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

flatten all level #6844

Merged
merged 10 commits into from
Apr 25, 2024
Merged

flatten all level #6844

merged 10 commits into from
Apr 25, 2024

Conversation

KimBioInfoStudio
Copy link
Contributor

@KimBioInfoStudio KimBioInfoStudio commented Apr 2, 2024

the original code only flatten 2d-dict, if there are some dict more than 2d, hard to qurey data and viz

TODO:

  • add a option btn to enable/disable this feature

Copy link

request-info bot commented Apr 2, 2024

We would appreciate it if you could provide us with more info about this issue/pr!

Copy link

codecov bot commented Apr 3, 2024

Codecov Report

Attention: Patch coverage is 66.66667% with 3 lines in your changes are missing coverage. Please review.

Project coverage is 63.41%. Comparing base (af0773c) to head (1593d7a).
Report is 32 commits behind head on master.

❗ Current head 1593d7a differs from pull request most recent head f15515d. Consider uploading reports for the commit f15515d to get more accurate results

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #6844      +/-   ##
==========================================
- Coverage   63.82%   63.41%   -0.41%     
==========================================
  Files         161      163       +2     
  Lines       13060    13209     +149     
  Branches     1803     1825      +22     
==========================================
+ Hits         8335     8377      +42     
- Misses       4425     4533     +108     
+ Partials      300      299       -1     
Files Coverage Δ
redash/query_runner/mongodb.py 40.47% <66.66%> (-0.59%) ⬇️

... and 11 files with indirect coverage changes

@justinclift
Copy link
Member

@KimBioInfoStudio Heya, there's no description for this PR. 😉

Um... why is flattening stuff useful for MongoDB?

@KimBioInfoStudio
Copy link
Contributor Author

KimBioInfoStudio commented Apr 3, 2024

@KimBioInfoStudio Heya, there's no description for this PR. 😉

Um... why is flattening stuff useful for MongoDB?

in the original code, only 2d dict was faltten, but my data struct have 4d dict like, it's hard for me to query on low level and viz on low level:

{ 
  "a": "aa",
  "b": ["bb1","bb2","bb3","bb4"],
  "c": {
      "cc1": ["cc11", "cc12", "cc13"],
      "cc2": {"cc21k":"cc21v"}
  }
}

@justinclift
Copy link
Member

@AndrewChubatiuk @eradman What do you both think of this?

I'm concerned that this will potentially break things for existing users of MongoDB.

If that's not likely to happen though, then this might be ok. 😄

@KimBioInfoStudio
Copy link
Contributor Author

@AndrewChubatiuk @eradman What do you both think of this?

I'm concerned that this will potentially break things for existing users of MongoDB.

If that's not likely to happen though, then this might be ok. 😄

u r right, if the option is not configureable, this will break existing mongodb users

@justinclift
Copy link
Member

justinclift commented Apr 3, 2024

That sounds like a problem. 😉

The option button might be a good way to go. It would need to default to the old behaviour, so people can upgrade without fear. 😄

@KimBioInfoStudio KimBioInfoStudio marked this pull request as draft April 7, 2024 03:19
@KimBioInfoStudio KimBioInfoStudio marked this pull request as ready for review April 7, 2024 07:29
@AndrewChubatiuk
Copy link
Contributor

Hey @KimBioInfoStudio
Nice feature
LGTM

@justinclift justinclift enabled auto-merge (squash) April 25, 2024 11:11
@justinclift
Copy link
Member

Thanks for completing this @KimBioInfoStudio, and thanks @AndrewChubatiuk for reviewing. 😄

Copy link
Member

@justinclift justinclift left a comment

Choose a reason for hiding this comment

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

Going with @AndrewChubatiuk's review conclusion. 😉

@justinclift justinclift merged commit 0624471 into getredash:master Apr 25, 2024
11 checks passed
@KimBioInfoStudio KimBioInfoStudio deleted the kim/mongo_flatten_dict branch April 26, 2024 05:43
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.

3 participants