PDF import from tastytrade and tastyworks #3493

stockmarketpotato · 2023-08-08T08:26:01Z

Issue: https://forum.portfolio-performance.info/t/pdf-import-from-tastytrade/24238

PDF import for tastytrade Trade Confirmations and Account Statements including support for Options trading and option's symbols following the OCC symbology standard.

Imports

Stock trades
Option trades
Option expiration and assignment
Dividends and interest payments
Debit interest
Funds paid and received
Journal to other accounts

…res for Options contracts.

Nirus2000 · 2023-08-09T13:49:31Z

Hello @stockmarketpotato
Thanks for this pull request.

I have had a little look at the source. We always try (95% all cases) to generate a unified source in the PDF importers. For this we have developed the Contributing Rules. We ask you to read them again and adjust your source code.
Please have a look at the examples in the Contributing rules and implement the structure.

A good idea is, replace all [ ]+ with [\\s]+ if more than one space can occur. Otherwise a simple space .

I think the OccOsiSymbology.java has to be integrated into the PDF importer TastytradePDFExtractor.java, because it is used exclusively for this PDF importer. What do you think?

For taxes and fees, use the V-Bank Importer's identification of the addTaxesSectionsTransaction and addFeesSectionsTransaction.
Could you imagine using them?

All in all, this is a good pull request and is not meant to be a criticism. 👍🏻
Good Job!

I would check again after your customization.
Therefore, @buchen please put in "draft" again.

Regards
Alex

stockmarketpotato · 2023-08-10T09:16:26Z

Dear Alex,
thanks for your fast review and your detailed guidance!😃

Please have a look at the examples in the Contributing rules and implement the structure.

Sorry, I was a bit too sloppy with the rules. The implementation puts more focus on functionality due to a tight time budget on my side. I will read the Contributing rules and implement the proposed structure.

I think the OccOsiSymbology.java has to be integrated into the PDF importer TastytradePDFExtractor.java, because it is used exclusively for this PDF importer. What do you think?

It depends on the relevance of options trading for PP and whether it is useful to other importers.
I will merge it back into the importer. So we have that clean for now. It can be moved out into a separate file later, if needed.

For taxes and fees, use the V-Bank Importer's identification of the addTaxesSectionsTransaction and addFeesSectionsTransaction. Could you imagine using them?

I have no experience with Generic Types in Java yet. This is a good chance to learn about it and reduce some of the redundancies in the code.

All in all, this is a good pull request and is not meant to be a criticism. 👍🏻 Good Job!

Thank you 😊

I would check again after your customization. Therefore, @buchen please put in "draft" again.

I will try to provide the updated code by the end of CW33.

BR
Sebastian

pfalcon · 2023-08-10T10:11:32Z

It depends on the relevance of options trading for PP

Indeed, would be interesting to hear @buchen's opinion on whether option trading support might be added to PP (of course, support for short trades is on critical path for this).

In either case, in the name of #3303 , I'd suggest to keep OccOsiSymbology.java separate for possible future reuse. This would be one small step towards growing something new cool in PP (like that option support). Of course, very small step, but dozens of such steps done by different people might lead somewhere. Whereas merging it into the importer won't lead anywhere by code duplication (next guy who'll need to deal with option symbols of course won't fish for it in another obscure importer, just write duplicate code again).

Nirus2000 · 2023-08-10T18:34:00Z

Hello @stockmarketpotato

Sorry, I was a bit too sloppy with the rules. The implementation puts more focus on functionality due to a tight time budget on my side. I will read the Contributing rules and implement the proposed structure.

No problem... Sometimes I catch myself and have to do it again. The good thing about it is that through these repetitions of the source a quick understanding comes and then global changes can also be implemented very quickly. For example with function outsourcing.

It depends on the relevance of options trading for PP and whether it is useful to other importers.
I will merge it back into the importer. So we have that clean for now. It can be moved out into a separate file later, if needed.

The idea of implementing options has been around for a long time, but it's just not that easy to implement. We have been thinking about how best to implement this for several months now and are in the process of identifying the problems and collecting them analytically. Including options simply with "Put &Call" is simply a whole new booking type with a cross behavior and must be considered separately. (Reference account <-> securities account)
We would put the source for this in the "code wallet" to access it back later....
Could you think of pushing the pull request without the options for now? (maybe possible - your choise)

I have no experience with Generic Types in Java yet. This is a good chance to learn about it and reduce some of the redundancies in the code.

I can understand that... 😄 but you can do it. 💪 💯
If you have any questions... always here, we are happy about energetic contributors, who work permanently and continuously on PP.

Regards... and many thx
Alex

buchen · 2023-08-14T07:37:56Z

I'd suggest to keep OccOsiSymbology.java separate for possible future reuse

I agree. If it is not specific to a one importer, place it where it is reusable.

About options in general: As I do not fully understand the implications, my strategy at the moment is: do not prevent the way options are currently maintained, but also do not introduce partial functionality that might required incompatible migration in the future. Personally, I just do not understand enough of the financial mathematics involved.

stockmarketpotato · 2023-08-15T14:04:07Z

Please have a look at the examples in the Contributing rules and implement the structure.

The structure is now reworked and comments have been added to make the code easier to understand. There are only two different types of documents. The entry points for the Trade Confirmation is
addSummaryStatementBuySellTransaction() and the entry point for the Account Statement (aka Kontoauszug) is addDepotStatementTransaction(). The function addDividendTransaction() adds the dividend transactions which are also contained in the Account Statement. It could be added to addDepotStatementTransaction(), if you find it more suitable.
Names of detected values are updated.
There are some new test cases to cover options expiration and assignment.
I was able to figure out one place to use it. Do you think it makes sense?

portfolio/name.abuchen.portfolio/src/name/abuchen/portfolio/datatransfer/pdf/TastytradePDFExtractor.java

Line 485 in 5485f04

private <T extends Transaction<?>> void addInterestsSectionTransaction(T transaction, DocumentType type)
Do you see some other possible points for improvement?

Including options simply with "Put &Call" is simply a whole new booking type with a cross behavior and must be considered separately. (Reference account <-> securities account)

I lack the knowledge of the complete code base to make any reasonable proposal here. However, Options can be treated like any other Security in PP. They have an identification number, a name and a couple of distinctive properties. Maybe they could be added as attributes?
There are two properties which are special in some sense. One option contract contains 100 shares. That's why the importer multiplies with 100. That could be done under the hood.
The other property is expiration. A contract is removed from the account when it expires. This transaction appears in the account statement anyways. This is implemented in the current importer with a buy or sell transaction depending on the position size (long, short) with zero cost. Would a removal do the same thing, but more simple?
It is also very common to hold a short position and collect the premium (thetagang style) instead of betting on a long position. So it would not be necessary to warn on negative account balance for options.

Could you think of pushing the pull request without the options for now?

I'd prefer to keep the options. The options are added as a Security, like any other, and they do not need any special treatment. Quotes can be obtained from Yahoo using the OCC symbol.

I agree. If it is not specific to a one importer, place it where it is reusable.

Ok, I'll leave it in the utilities folder for now.

Nirus2000

What do you think about some suggested changes?
That could make the source code clearer, right?

name.abuchen.portfolio/src/name/abuchen/portfolio/datatransfer/pdf/TastytradePDFExtractor.java

Nirus2000 · 2023-08-19T07:30:05Z

name.abuchen.portfolio/src/name/abuchen/portfolio/datatransfer/pdf/TastytradePDFExtractor.java

+                    t.setShares(asShares(v.get("shares"), "en", "US"));
+                    v.put("currency", "USD");
+                    Money tax = Money.of("USD", asAmount(v.get("tax"), "en", "US"));
+                    ExtractorUtils.checkAndSetTax(tax, t, type.getCurrentContext());


ExtractorUtils.checkAndSetTax(tax, t, type.getCurrentContext());

Maybe you use this method in the same example... see in the other PDF importer
(addTaxesSectionsTransaction / addFeesSectionsTransaction)

Can you elaborate this a bit more?

You could simply look at the other PDF importers and adopt their structure. You will find that they are always structured in the same way in order to achieve standardization. :-)

Nirus2000 · 2023-08-19T07:38:09Z

name.abuchen.portfolio/src/name/abuchen/portfolio/datatransfer/pdf/TastytradePDFExtractor.java

+        pdfTransaction.subject(() -> {
+            AccountTransaction entry = new AccountTransaction();
+            entry.setType(AccountTransaction.Type.DIVIDENDS);
+            entry.setCurrencyCode("USD");


Maybe it's better they extract the currency globally from the document instead of setting it manually over and over again.

final DocumentType type = new DocumentType("XXXXXXX", (context, lines) -> { Pattern pCurrency = Pattern.compile("^.* XXXXXX$, (?<currency>[\\w]{3})$"); for (String line : lines) { Matcher mCurrency = pCurrency .matcher(line); if (mCurrency.matches()) context.put("currency", mCurrency.group("currency")); break; } }); this.addDocumentTyp(type);

After that you can pick them up in the individual sections

.assign((t, v) -> { Map<String, String> context = type.getCurrentContext(); v.put("currency", context.get("currency")); ... })

The currency is always USD. IMHO no change is required.
Is there a way to place it by default in ParsedData?
Is it necessary to always set the currency in the transaction?

Nirus2000 · 2023-08-19T07:40:14Z

name.abuchen.portfolio/src/name/abuchen/portfolio/datatransfer/pdf/TastytradePDFExtractor.java

+                Money fee = Money.of("USD", asAmount(v.get("commfee"), "en", "US")
+                                + asAmount(v.get("tranfee"), "en", "US")
+                                + asAmount(v.get("addlfee"), "en", "US"));


Would be more helpful to calculate an amount via the Money.class.
This way we have the check that the currencies are also correct to each other.

The currency is always USD. Internally, Money uses a long member to represent the total amount. The method asAmount(..) will return a long. That means using the Money class should not make any difference, computation-wise. But it would be more clear and readable though. The current version is just shorter.
We will do whatever you prefer.

Nirus2000 · 2023-08-19T07:41:27Z

name.abuchen.portfolio/src/name/abuchen/portfolio/datatransfer/pdf/TastytradePDFExtractor.java

+            .match("^(?<description>.*).*$")
+            .match("(^OPTION EXPIRATION.*$|^.*[\\s]+[A-Z0-9]+[\\s]+[\\d]+[\\s]+ASSIGNED.*)")
+            .match("Security Number:[\\s]+(?<cusip>[A-Z0-9]+)")
+            .assign((t, v) -> {                v.put("currency", "USD");


here is a problem...

I've converted the file and changed the git configuration as mention on Stackoverflow.
How can I check the line endings?

Nirus2000 · 2023-08-19T07:46:39Z

name.abuchen.portfolio/src/name/abuchen/portfolio/datatransfer/pdf/TastytradePDFExtractor.java

+                if (v.get("type").equals("EXPIRED")) {
+                    setTypeAndExpirationNote(t, v); // TODO
+                } else {
+                    t.setNote("Assigned: Buy To Close");
+                    t.setType(PortfolioTransaction.Type.BUY); // Assignment means this was a short position
+                }


In this function you already set the type "Buy" above.
So if the type should change, then you could, if you want, use the following code

Maybe it's better to use a separate section like

.section("type").optional() .match("^XXXXX (?<type>YYYYYYY)$") .assign((t, v) -> { if ("YYYYYYY".equals(v.get("type"))) t.setType(PortfolioTransaction.Type.SELL); })

Depending on the type (expiration or assignment), different transactions are possible.

Long and short positions can expire "worthless" out of the money. That means whenever we see an expiration transaction in the Account Statement, we need to check whether it was a long or a short position at expiration.

Only short positions can be assigned, which happens when the contract expires in the money. So we do not need to check the quantity.

Here is a hopefully more readable and understandable version.

Suggested change

if (v.get("type").equals("EXPIRED")) {

setTypeAndExpirationNote(t, v); // TODO

} else {

t.setNote("Assigned: Buy To Close");

t.setType(PortfolioTransaction.Type.BUY); // Assignment means this was a short position

}

// Expiration means it can be a long or short position that expired OTM

if (v.get("type").equals("EXPIRED")) {

BigDecimal quantity = ExtractorUtils.convertToNumberBigDecimal(v.get("quantity"), Values.Amount, "en", "US");

if (quantity.signum() == 1) {

t.setType(PortfolioTransaction.Type.BUY);

t.setNote("Expired: Buy To Close");

}

else if (quantity.signum() == -1) {

t.setType(PortfolioTransaction.Type.SELL);

t.setNote("Expired: Sell To Close");

}

// Assignment means this was a short position

} else if (v.get("type").equals("ASG")) {

t.setNote("Assigned: Buy To Close");

t.setType(PortfolioTransaction.Type.BUY);

}

Nirus2000 · 2023-08-19T07:48:06Z

name.abuchen.portfolio/src/name/abuchen/portfolio/datatransfer/pdf/TastytradePDFExtractor.java

+                                Double.parseDouble(v.get("strike").toString()));
+                v.put("tickerSymbol", o.getOccKey());
+                v.put("name", o.getName());
+                t.setShares(asShares(v.get("quantity").replace("-", ""), "en", "US") * 100);


I think it is better to use BigDecimal for the calculation of the quantity...

Suggested change

t.setShares(asShares(v.get("quantity").replace("-", ""), "en", "US") * 100);

BigDecimal shares = asBigDecimal(v.get("shares"));

t.setShares(Values.Share.factorize(shares.doubleValue() * 100));

stockmarketpotato · 2023-08-19T08:07:51Z

@Nirus2000 thanks for your detailed suggestions. I will implement them all.

About this ^M thing, I have no clue what's going on there 🦧

Nirus2000 · 2023-08-19T08:26:56Z

you need to upload in UNIX style.
Maybe this helps you https://stackoverflow.com/questions/1889559/make-git-diff-ignore-m

Nirus2000 · 2023-08-19T08:32:42Z

name.abuchen.portfolio/src/name/abuchen/portfolio/datatransfer/pdf/TastytradePDFExtractor.java

+    private LocalDateTime asDate(String dateString)
+    {
+        DateTimeFormatter formatter = DateTimeFormatter.ofPattern("M/d/yy").withLocale(Locale.US);
+        return LocalDate.parse(dateString, formatter).atStartOfDay();
+    }
+}


You can remove this part if you add the date format in ExtractorUtils.java.
see --> DATE_FORMATTER_US ...

Ah! Did not see this before.
Done ✔

Nirus2000 · 2023-09-06T10:17:30Z

name.abuchen.portfolio/src/name/abuchen/portfolio/datatransfer/pdf/TastytradePDFExtractor.java

+                    BigDecimal quantity = ExtractorUtils.convertToNumberBigDecimal(v.get("quantity"),
+                                    Values.Amount, "en", "US");


change to this like the other override commands

@Override protected BigDecimal asExchangeRate(String value) { return ExtractorUtils.convertToNumberBigDecimal(value, Values.Share, "en", "US"); }

Nirus2000 · 2023-09-06T10:22:34Z

Hello @stockmarketpotato
I could imagine that you look at the other PDF importers again to internalize the structure as well as the hints we gave you. This could help us to better maintain the source code later when changes are made by other contributors.

Especially the comment from the main developer should make it clear to you that we don't want to process any options at the moment.

What do you think about it?

Regards
Alex

stockmarketpotato · 2023-09-11T08:43:20Z

Especially the comment from the main developer should make it clear to you that we don't want to process any options at the moment.

Hi Alex, I must have misunderstood the comment. It was not that clear to me. Especially since IBFlex importer does in fact process and import option trades

portfolio/name.abuchen.portfolio/src/name/abuchen/portfolio/datatransfer/ibflex/IBFlexStatementExtractor.java

Line 737 in 5519b5c

// e.g a put option for oracle: ORCL 171117C00050000

What we can do is separate the options transactions in the importer, e.g. in separate functions or generics, and integrate them at a later point in time. What do you think?

Cheers
Sebastian

stockmarketpotato · 2024-03-01T13:59:27Z

Hi @Nirus2000, @buchen, it's been some time since there has been any activity or updates from my end, and I appreciate your patience and the time you've already invested in reviewing it.

I wanted to propose removing all parts related to stock options to potentially make it more aligned with the project's goals. Would this change address any concerns and make the PR more acceptable?

I value your feedback and am open to any suggestions you might have. Looking forward to hearing from you.

BR
Sebastian

seg-on · 2024-06-19T07:25:49Z

Hi, is there any news on this topic? :)

stockmarketpotato · 2024-07-05T07:42:40Z

@seg-on , have you been able to compile the branch and run it for your documents?
I have not been able to work on this PR recently. It has fallen behind main branch and merge conflicts must be resolved before continuing further. Do you want to work together on this PR?

Nirus2000 · 2024-07-05T08:05:50Z

name.abuchen.portfolio/src/name/abuchen/portfolio/datatransfer/pdf/TastytradePDFExtractor.java

+                    t.setShares(asShares(v.get("shares"), "en", "US"));
+                    v.put("currency", "USD");
+                    Money tax = Money.of("USD", asAmount(v.get("tax"), "en", "US"));
+                    ExtractorUtils.checkAndSetTax(tax, t, type.getCurrentContext());


You could simply look at the other PDF importers and adopt their structure. You will find that they are always structured in the same way in order to achieve standardization. :-)

seg-on · 2024-07-09T10:10:41Z

@stockmarketpotato Sorry, but I'm not a Java programmer. I can't help you. :/

stockmarketpotato added 3 commits July 31, 2023 21:32

First version of tastytrade PDF importer and test cases.

f72d9c8

Move OccOsiSymbology to own class under utils. Fix calculation of sha…

02cdaba

…res for Options contracts.

Merge branch 'portfolio-performance:master' into tasty-import

6fe37d4

buchen assigned Nirus2000 Aug 9, 2023

Nirus2000 self-requested a review August 9, 2023 13:30

Nirus2000 added the pdf label Aug 9, 2023

Nirus2000 removed their request for review August 9, 2023 13:57

Updated structure and new test cases.

5485f04

Fix Maven Build: set Locale.US for strike price format.

23f530b

Nirus2000 reviewed Aug 19, 2023

View reviewed changes

stockmarketpotato added 5 commits September 5, 2023 14:49

Implemented suggested code improvements from @Nirus2000

cc9f7b2

Minimal code cosmetics

a8c2a57

Merge branch 'portfolio-performance:master' into tasty-import

c8f0007

Remove unused imports

1e5988b

Fix formatting to satisfy style check

82870bb

Nirus2000 reviewed Sep 6, 2023

View reviewed changes

Nirus2000 requested changes Jul 5, 2024

View reviewed changes

Nirus2000 marked this pull request as draft July 5, 2024 08:07

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

PDF import from tastytrade and tastyworks #3493

PDF import from tastytrade and tastyworks #3493

stockmarketpotato commented Aug 8, 2023

Nirus2000 commented Aug 9, 2023 •

edited

Loading

stockmarketpotato commented Aug 10, 2023

pfalcon commented Aug 10, 2023

Nirus2000 commented Aug 10, 2023

buchen commented Aug 14, 2023

stockmarketpotato commented Aug 15, 2023

Nirus2000 left a comment

Nirus2000 Aug 19, 2023

stockmarketpotato Sep 5, 2023

Nirus2000 Jul 5, 2024

Nirus2000 Aug 19, 2023 •

edited

Loading

stockmarketpotato Sep 5, 2023

Nirus2000 Aug 19, 2023

stockmarketpotato Sep 5, 2023

Nirus2000 Aug 19, 2023

stockmarketpotato Sep 5, 2023

Nirus2000 Aug 19, 2023

stockmarketpotato Sep 5, 2023

Nirus2000 Aug 19, 2023 •

edited

Loading

stockmarketpotato Sep 5, 2023

stockmarketpotato commented Aug 19, 2023

Nirus2000 commented Aug 19, 2023

Nirus2000 Aug 19, 2023

stockmarketpotato Sep 5, 2023

Nirus2000 Sep 6, 2023

Nirus2000 commented Sep 6, 2023

stockmarketpotato commented Sep 11, 2023

stockmarketpotato commented Mar 1, 2024

seg-on commented Jun 19, 2024

stockmarketpotato commented Jul 5, 2024

Nirus2000 Jul 5, 2024

seg-on commented Jul 9, 2024

	t.setShares(asShares(v.get("quantity").replace("-", ""), "en", "US") * 100);
	BigDecimal shares = asBigDecimal(v.get("shares"));
	t.setShares(Values.Share.factorize(shares.doubleValue() * 100));

		BigDecimal quantity = ExtractorUtils.convertToNumberBigDecimal(v.get("quantity"),
		Values.Amount, "en", "US");

PDF import from tastytrade and tastyworks #3493

Are you sure you want to change the base?

PDF import from tastytrade and tastyworks #3493

Conversation

stockmarketpotato commented Aug 8, 2023

Nirus2000 commented Aug 9, 2023 • edited Loading

stockmarketpotato commented Aug 10, 2023

pfalcon commented Aug 10, 2023

Nirus2000 commented Aug 10, 2023

buchen commented Aug 14, 2023

stockmarketpotato commented Aug 15, 2023

Nirus2000 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Nirus2000 Aug 19, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Nirus2000 Aug 19, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

stockmarketpotato commented Aug 19, 2023

Nirus2000 commented Aug 19, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Nirus2000 commented Sep 6, 2023

stockmarketpotato commented Sep 11, 2023

stockmarketpotato commented Mar 1, 2024

seg-on commented Jun 19, 2024

stockmarketpotato commented Jul 5, 2024

Choose a reason for hiding this comment

seg-on commented Jul 9, 2024

Nirus2000 commented Aug 9, 2023 •

edited

Loading

Nirus2000 Aug 19, 2023 •

edited

Loading

Nirus2000 Aug 19, 2023 •

edited

Loading