Add missing attributes #2
Conversation
Reviewer's Guide by SourceryThis pull request adds several missing attributes to the File-Level Changes
Tips
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey @Trefex - I've reviewed your changes and they look great!
Here's what I looked at during the review
- 🟡 General issues: 4 issues found
- 🟢 Security: all looks good
- 🟢 Testing: all looks good
- 🟢 Complexity: all looks good
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment to tell me if it was helpful.
| cusip: Optional[str] = None | ||
| isin: Optional[str] = None | ||
| rtn: Optional[str] = None | ||
| initialInvestment: Optional[str] = None |
There was a problem hiding this comment.
suggestion: Consider using snake_case for consistency.
The rest of the attributes in the class use snake_case. It would be more consistent to name this attribute initial_investment.
| initialInvestment: Optional[str] = None | |
| initial_investment: Optional[str] = None |
| weight: Optional[str] = None | ||
| figi: Optional[str] = None | ||
|
|
||
| issuerCountryCode: Optional[str] = None |
There was a problem hiding this comment.
issue (bug_risk): Duplicate attribute in CashTransaction class.
The issuerCountryCode attribute is already defined in the CashTransaction class. This could lead to confusion or unintended behavior.
| figi: Optional[str] = None | ||
|
|
||
| issuerCountryCode: Optional[str] = None | ||
| availableForTradingDate: Optional[datetime.date] = None |
There was a problem hiding this comment.
suggestion: Consider using snake_case for consistency.
The rest of the attributes in the class use snake_case. It would be more consistent to name this attribute available_for_trading_date.
| availableForTradingDate: Optional[datetime.date] = None | |
| available_for_trading_date: Optional[datetime.date] = None |
| """ | ||
| ASSIGNMENT = "A" | ||
| AUTOEXERCISE = "AEx" # Automatic exercise for dividend-related recommendation | ||
| AUTOFX = "AFx" # FX Auto Conversion |
There was a problem hiding this comment.
suggestion: Consider using consistent casing for enum values.
The other enum values use uppercase letters only. Consider changing AFx to AFX for consistency.
| AUTOFX = "AFx" # FX Auto Conversion | |
| AUTOFX = "AFX" # FX Auto Conversion |
|
@agusalex is this fork still supported? |
The only purpose of this fork is to contribute to https://github.com/csingley/ibflex |
3 participants
Add missing attributes
Summary by Sourcery
This pull request introduces new attributes to the Trade and CashTransaction classes, and adds a new enum value to the Code enum to enhance the data model.