Skip to content

Fix dtype inference and session cookie handling #1237

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Fix dtype inference and session cookie handling #1237

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
fafdb3f
fix: set default dtype to str in pd.read_csv to prevent unintended ty…
Cubewise-Ronan Apr 9, 2025
cac555d
fix: multiple cookies with name, 'TM1SessionId'
Cubewise-Ronan Apr 9, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 16 additions & 2 deletions TM1py/Services/RestService.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -786,8 +786,22 @@ def _start_session(self, user: str, password: str, decode_b64: bool = False, nam
finally:
# If the TM1 REST API is routed through a reverse proxy that alters the expected URL,
# we explicitly re-set the 'TM1SessionId' cookie to maintain session continuity.
session_id = self._s.cookies.pop('TM1SessionId', None)
if session_id is not None:
session_id = None
for cookie in self._s.cookies:
if cookie.name == 'TM1SessionId':
session_id = cookie.value
# break # Use the first match
Copy link
Collaborator

Choose a reason for hiding this comment

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

Maybe this is an obsolete comment or did you want to break after the first match?


# Clear all TM1SessionId cookies to prevent duplicates
cookies_to_remove = [
(cookie.domain, cookie.path)
for cookie in self._s.cookies
if cookie.name == 'TM1SessionId'
]
for domain, path in cookies_to_remove:
self._s.cookies.clear(domain=domain, path=path, name='TM1SessionId')

if session_id:
self._s.cookies.set('TM1SessionId', session_id)

# After we have session cookie, drop the Authorization Header
Expand Down
2 changes: 1 addition & 1 deletion TM1py/Utils/Utils.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -545,7 +545,7 @@ def build_dataframe_from_csv(raw_csv, sep='~', shaped: bool = False,

# make sure all element names are strings and values column is derived from data
if 'dtype' not in kwargs:
kwargs['dtype'] = {'Value': None, **{col: str for col in range(999)}}
kwargs['dtype'] = {'Value': str, **{col: str for col in range(999)}}
Copy link
Collaborator

Choose a reason for hiding this comment

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

We don't want the value column to be str in all cases, do we?

If we set it to None instead, it will be derived depending on the data at hand

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Actually, that was the issue I was running into — setting 'Value': None causes pandas to infer the dtype, and in some cases like "2025" (which is an attribute value for an element and should be a string), it gets coerced to 2025.0 if other rows are missing values or inconsistent.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I understand. But we can't enforce str as the column type for the value column. Most retrieved cube data will be numeric!

In the edge cases where you load data from attribute cubes it would be easier to pass dtype=str to the execute_mdx_dataframe function.

try:
df = pd.read_csv(StringIO(raw_csv), sep=sep, na_values={'Value': ['None']}, keep_default_na=False, **kwargs)

Expand Down