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.

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

Conversation

Cubewise-Ronan
Copy link
Contributor

  • Set default dtype to str in pd.read_csv to avoid misinterpreting values like '2025' as floats.
  • Fixed issue with multiple 'TM1SessionId' cookies causing CookieConflictError during session start.
File "C:\Dev\share_service\venv\lib\site-packages\TM1py\Services\RestService.py", line 789, in _start_session
    session_id = self._s.cookies.pop('TM1SessionId', None)
  File "C:\Program Files\Python310\lib\_collections_abc.py", line 954, in pop
    value = self[key]
  File "C:\Dev\share_service\venv\lib\site-packages\requests\cookies.py", line 334, in __getitem__
    return self._find_no_duplicates(name)
  File "C:\Dev\share_service\venv\lib\site-packages\requests\cookies.py", line 405, in _find_no_duplicates
    raise CookieConflictError(
requests.cookies.CookieConflictError: There are multiple cookies with name, 'TM1SessionId'

@@ -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.

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?

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.

2 participants