Skip to content

Conversation

@elho
Copy link
Contributor

@elho elho commented May 7, 2025

The values for daily low and high values as well as trade volume are typically provided within the server response (nowadays, I guess), so use them, if they are present. The code does handle them missing and the old tests with server responses without these extra attributes are left in place to test this.

Also included is a small cleanup of the tests, they defined security objects that were never used and in some case used a completely different ticker symbol than the test data. I assume that being a result of some too generous copy & paste from other tests, so dropped them.

@Morpheus1w3
Copy link
Contributor

@elho
Let me ask one question, for what purpose? Nothing in PP is calculated with it? This will just increase the file size and decrease the performance, as everything is loaded into the memory at runtime.

@elho
Copy link
Contributor Author

elho commented May 8, 2025

Let me ask one question, for what purpose? Nothing in PP is calculated with it? This will just increase the file size and decrease the performance, as everything is loaded into the memory at runtime.

The data source tab of edit security dialog (where you select the data source) has entries for low, high and volume in the table that remain empty/zero otherwise, and the graph tab at the bottom of the securities list on the right hand side shows the (latest) High, Low and Volume (showing as n/a otherwise).
In the portfolio file the low, high, and Volume values apparently are stored only for the latest price. (And the XML tags for them, which typically take up more space than the actual values, are always present anyway.) So you will be hard pressed to even see a difference looking at the file size when using human readable numbers.

@elho elho force-pushed the feature/yahoofnance_low_high_volume branch from 4864f74 to 83a3ab9 Compare May 12, 2025 10:54
@sumansuhag

This comment was marked as spam.

@buchen buchen merged commit 6cda98d into portfolio-performance:master Jul 12, 2025
2 checks passed
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.

4 participants