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

Maika 1033 #1051

Merged
merged 4 commits into from
Oct 10, 2023
Merged

Maika 1033 #1051

merged 4 commits into from
Oct 10, 2023

Conversation

ntran18
Copy link
Collaborator

@ntran18 ntran18 commented Sep 24, 2023

Fixing bottom dataset

@ntran18 ntran18 changed the base branch from Sarronnn to beta September 24, 2023 04:22
@ntran18
Copy link
Collaborator Author

ntran18 commented Sep 24, 2023

I was able to make the selection in the bottom data menu. However, I'm thinking there is problem with database. One notice problem is that in graph.js, grnState.workbook.expression[dataset] not always contains data, it can be a null object. I fixed this one, however, sometimes I choose a random expression, the error shows there is no value p.length but doesn't show which files it refer to. I wasn't able to look more on this problem this week because I didn't have time. Will take a look at it again next week.

Copy link
Owner

@dondi dondi left a comment

Choose a reason for hiding this comment

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

Looks like this PR has linter issues—for direct reference, you can look at the Travis report: https://app.travis-ci.com/github/dondi/GRNsight/builds/266145719

You should also be able to do npm run lint on the command line in order to see these messages

@dondi
Copy link
Owner

dondi commented Sep 26, 2023

As a side note, these linter errors will be more avoidable if we configure an autoformatter that complies with the current style checks made by the npm run lint command. We have an issue for this (#882) which we can bump in priority when the time is right

@ntran18
Copy link
Collaborator Author

ntran18 commented Oct 9, 2023

I expected some errors would happen. However, these errors are out of my scope of understand so I might need to meet with @dondi to discuss about these problems. I also think this is a different issue, related to database, not the UI.

Problems: When you click in some expressions, especially the one contains ACE node, it would not reflect on the UI, meaning the check doesn't change to your selection. This is because the program can't load the expression. As I mentioned before, this is because there is no value p.length (I experienced some expressions, and it happens mostly for ACE node, not sure about the other nodes). However, if you click on that selection again, it works, but I don't know if it is correct or not.

Demo:

GRNsight_reduced.mov
  1. Lint errors
image image image image --> From what I understand, I think either the version of database I currently have has different expression sheet name. A lot of errors were wrong name, or worksheet not detected.
  1. Example of error from console
image

Copy link
Owner

@dondi dondi left a comment

Choose a reason for hiding this comment

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

LGTM from a code perspective; but as noted in #1033 and in the comments, merging these changes appears to reveal an internal error due to missing data

@dondi dondi merged commit 97e3887 into beta Oct 10, 2023
@dondi dondi deleted the maika-1033 branch October 17, 2023 16:05
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