Mock API for retrieving data from simulation run#96
Mock API for retrieving data from simulation run#96hsorby wants to merge 6 commits intoABI-Software:mainfrom
Conversation
src/components/SimulationVuer.vue
Outdated
| return response | ||
| } | ||
|
|
||
| defineExpose({ getDataView }) |
There was a problem hiding this comment.
Why getDataView? Why not simply getData?
| </template> | ||
|
|
||
| <script setup> | ||
| const emit = defineEmits(['data-ready']) |
There was a problem hiding this comment.
I would use data-available (or dataAvailable).
There was a problem hiding this comment.
In fact, this emit is not used, so why define it?
| if (type === 'sine') { | ||
| // Smooth curve. | ||
| data.push(Math.sin(i * 0.1) * 10) | ||
| } else { | ||
| // Random noise. | ||
| data.push(Math.random() * 10) | ||
| } |
There was a problem hiding this comment.
Might want to add some mock data for the variable of integration?
| // Check request is expected type. | ||
| if (req.id !== EXPECTED_ID) { | ||
| console.warn(`[Mock] Request #${index} ignored: Invalid ID '${req.id}'`) | ||
| return // Skip this specific item. | ||
| } | ||
|
|
||
| // Test version compatibility. | ||
| const requestMajorVersion = parseInt(req.version.split('.')[0]) | ||
| if (requestMajorVersion !== EXPECTED_MAJOR_VERSION) { | ||
| console.warn(`[Mock] Request #${index} ignored: Version mismatch. Expected v${EXPECTED_MAJOR_VERSION}.x, got v${req.version}`) | ||
| return // Skip this specific item. | ||
| } |
There was a problem hiding this comment.
I wouldn't bother with this, but... meh.
There was a problem hiding this comment.
Still wouldn't bother with this kind of check.
| if (req.variable && req.variable.includes('v_in')) { | ||
| response[req.identifier] = generateMockSeries('sine') | ||
| } else { | ||
| // Default fallback for other variables. |
There was a problem hiding this comment.
Also come here if req.variable is not truthy.
| } else { | ||
| // Default fallback for other variables. | ||
| response[req.identifier] = generateMockSeries('random') | ||
| } |
There was a problem hiding this comment.
I would expect req.variable to be something like membrane/V or sodium_current/i_Na. As for the variable of integration, we could use VOI as a special variable name. So, here, I would expect to add a case for where req.variable === 'VOI'?
agarny
left a comment
There was a problem hiding this comment.
It would be nice to have a PR that doesn't consist of 99% of reformatted code. It makes the PR unnecessarily difficult to review.
This aside, I understand that you expose getData():
- I would probably called that method
getSimulationData(). - It's not clear to me what the
requestsparameter is supposed to look like.
| if (!requests || !Array.isArray(requests)) { | ||
| console.error("getDataView: Request must be an array.") | ||
| return {}; | ||
| console.error('getData: Request must be an array.') |
|
|
||
| function generateMockSeries(type, length = 100) { | ||
| const data = [] | ||
| for (let i = 0; i < length; i++) { |
There was a problem hiding this comment.
Very minor since this is bound to be removed, but: ++i rather than i++.
| // Check request is expected type. | ||
| if (req.id !== EXPECTED_ID) { | ||
| console.warn(`[Mock] Request #${index} ignored: Invalid ID '${req.id}'`) | ||
| return // Skip this specific item. | ||
| } | ||
|
|
||
| // Test version compatibility. | ||
| const requestMajorVersion = parseInt(req.version.split('.')[0]) | ||
| if (requestMajorVersion !== EXPECTED_MAJOR_VERSION) { | ||
| console.warn(`[Mock] Request #${index} ignored: Version mismatch. Expected v${EXPECTED_MAJOR_VERSION}.x, got v${req.version}`) | ||
| return // Skip this specific item. | ||
| } |
There was a problem hiding this comment.
Still wouldn't bother with this kind of check.
| </template> | ||
|
|
||
| <script setup> | ||
| const emit = defineEmits(['data-ready']) |
There was a problem hiding this comment.
In fact, this emit is not used, so why define it?
| requests.forEach((req, index) => { | ||
| // --- VALIDATION BLOCK --- | ||
|
|
||
| // Check request is expected type. |
There was a problem hiding this comment.
// Check that the request is of the expected type.
| const EXPECTED_ID = 'nz.ac.auckland.simulation-data-request' | ||
| const EXPECTED_MAJOR_VERSION = 0 | ||
|
|
||
| requests.forEach((req, index) => { |
There was a problem hiding this comment.
It's not 100% clear to me what requests is expected to look like.
|
|
||
| // Check request is expected type. | ||
| if (req.id !== EXPECTED_ID) { | ||
| console.warn(`[Mock] Request #${index} ignored: Invalid ID '${req.id}'`) |
There was a problem hiding this comment.
"invalid", not "Invalid". Also need a final stop.
| if (req.position) { | ||
| response.position = { ...req.position } | ||
| } |
There was a problem hiding this comment.
What is the point of req.position? It is in the request and then put in the response without doing anything with it in between?
| response.position = { ...req.position } | ||
| } | ||
|
|
||
| if (!response['data']) { |
There was a problem hiding this comment.
Isn't this test always true since we start with response = {}?
No description provided.