Skip to content

Add Python dict headers overload to SubscriptionDataSource#9363

Open
aiSynergy37 wants to merge 4 commits intoQuantConnect:masterfrom
aiSynergy37:fix-9361-subscriptiondatasource-pydict-headers
Open

Add Python dict headers overload to SubscriptionDataSource#9363
aiSynergy37 wants to merge 4 commits intoQuantConnect:masterfrom
aiSynergy37:fix-9361-subscriptiondatasource-pydict-headers

Conversation

@aiSynergy37
Copy link
Copy Markdown

Summary

  • add a SubscriptionDataSource constructor overload that accepts Python dict headers (PyObject)
  • convert Python dictionary headers into managed KeyValuePair<string, string> values
  • reject non-dictionary Python objects with a clear ArgumentException
  • add regression tests for valid, null, and invalid Python header inputs

Testing

  • unable to run local tests in this environment because dotnet is not installed

Closes #9361

/// <param name="headers">The Python dictionary containing the headers to be used for this source</param>
public SubscriptionDataSource(string source, SubscriptionTransportMedium transportMedium, FileFormat format, PyObject headers)
: this(source, transportMedium, format, ConvertHeaders(headers))
{
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

We have the ConvertToDictionary extension method we can use here:

 this(source, transportMedium, format, headers.ConvertToDictionary<string, string>())

By the way, we could use this method in QCAlgorithm.Python.cs#L1479 as well.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Done in d9548eb. I replaced the custom conversion with headers == null ? null : headers.ConvertToDictionary<string, string>() and updated the invalid-input test assertion to match the shared extension exception message.

Copy link
Copy Markdown
Member

@AlexCatarino AlexCatarino left a comment

Choose a reason for hiding this comment

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

Thank you, @aiSynergy37 ,
We should reuse the ConvertToDictionary extension method.

@aiSynergy37
Copy link
Copy Markdown
Author

Addressed review feedback in d9548eb: the PyObject headers overload now reuses ConvertToDictionary<string, string>() instead of custom conversion logic, and tests were updated accordingly.

@aiSynergy37
Copy link
Copy Markdown
Author

@AlexCatarino Thanks again. The ConvertToDictionary refactor is now in place on the latest commit (d9548eb). Could you please take another look when you have a moment?

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.

SubscriptionDataSource Ctor Overload for Headers as PyObject/Dict

3 participants